Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: ignore out of bound lines from ctags #694

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Conversation

keegancsmith
Copy link
Member

universal-ctags sometimes returns lines that are out of bounds. In practice it seems to only do an off by one. We haven't noticed the linenum error until a recent change of mine which didn't append an extra entry to NLS if the file was terminated by "\n". In practice this would end up being filtered out later on. So we update to just continue rather than error here.

An example is
https://github.com/sourcegraph/sourcegraph/blob/v5.2.2/client/web-sveltekit/.storybook/main.ts

$ universal-ctags '--fields=*' --output-format=json main.ts | grep 22
{"_type": "tag", "name": "config", "path": "main.ts", "pattern": "/^export default config$/", "language": "TypeScript", "line": 22, "kind": "constant", "roles": "def"}

$ wc -l main.ts
21 main.ts

$ tail -n1 main.ts
export default config

Test Plan: added a unit test

universal-ctags sometimes returns lines that are out of bounds. In
practice it seems to only do an off by one. We haven't noticed the
linenum error until a recent change of mine which didn't append an extra
entry to NLS if the file was terminated by "\n". In practice this would
end up being filtered out later on. So we update to just continue rather
than error here.

An example is
https://github.com/sourcegraph/sourcegraph/blob/v5.2.2/client/web-sveltekit/.storybook/main.ts

  $ universal-ctags '--fields=*' --output-format=json main.ts | grep 22
  {"_type": "tag", "name": "config", "path": "main.ts", "pattern": "/^export default config$/", "language": "TypeScript", "line": 22, "kind": "constant", "roles": "def"}

  $ wc -l main.ts
  21 main.ts

  $ tail -n1 main.ts
  export default config

Test Plan: added a unit test
@keegancsmith keegancsmith merged commit 137eb8f into main Nov 15, 2023
8 checks passed
@keegancsmith keegancsmith deleted the k/another-go branch November 15, 2023 09:10
@jtibshirani
Copy link
Member

Thanks! This helps explain why we didn't see a bunch of failures after deploying your fix ... scip-ctags does not have the same issue, and we use that by default for Typescript.

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.

3 participants