Skip to content

fix: interpret span correctly for github format#1621

Open
SoMuchForSubtlety wants to merge 4 commits into
googleapis:mainfrom
SoMuchForSubtlety:gh-format
Open

fix: interpret span correctly for github format#1621
SoMuchForSubtlety wants to merge 4 commits into
googleapis:mainfrom
SoMuchForSubtlety:gh-format

Conversation

@SoMuchForSubtlety

@SoMuchForSubtlety SoMuchForSubtlety commented Apr 9, 2026

Copy link
Copy Markdown

The old location logic had two bugs

  • keeping locations zero-indexed
  • interpreting end column as end line in case of span size 3

I decided to reuse the existing correct implementation from the lint package. I can re-implement it if you would prefer not to expose this API.

I also fixed newlines, github does not support \n, but at least partially supports %0A, see actions/toolkit#193

Closes #1071

@SoMuchForSubtlety SoMuchForSubtlety requested a review from a team as a code owner April 9, 2026 17:55

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors GitHub Actions output formatting by exporting and utilizing the FileLocation and Position types from the lint package. The manual span processing in github_actions.go is replaced with FileLocationFromPBLocation, and tests are updated to reflect the new output. Feedback includes suggestions to simplify formatting logic, correct a capitalization error in a comment, and improve documentation for the newly exported functions.

Comment thread cmd/api-linter/github_actions.go Outdated
Comment thread lint/problem.go Outdated
Comment thread lint/problem.go Outdated
@quirogas

Copy link
Copy Markdown
Contributor

Hey @SoMuchForSubtlety, thank you so much for submitting this PR. We definitely do not want to expose this API, so I think a small re-implementation would be appropriate. Additionally, if you don't mind, could you also reference #1071 to determine if this change addresses that issue?

Reuse location logic from the lint package

The old location logic had two bugs
- keeping locations zero-indexed
- interpreting end column as end line in case of span size 3
@SoMuchForSubtlety

Copy link
Copy Markdown
Author

@quirogas any chance of getting another review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitHub Action output format generates incorrect line numbers

2 participants