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

fix: fetch grammars from non-reference commits #133

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

jbedard
Copy link
Contributor

@jbedard jbedard commented Nov 30, 2023

Reference commits are often (but not always?) only references and not real commits that can be fetched with the githubusercontent.com URL.

This change removes the --refs filter so real commits are also included, which happen to come first with the current ls-remote sorting so it will use the real commit. This fixes updating of a lot of grammars which are broken today.

For example, with --refs:

> git ls-remote --tags --refs --sort -v:refname https://github.com/tree-sitter/tree-sitter-javascript | head -n5
dc2b73b62f162860cc8b55f51a21833ff98867e5	refs/tags/v0.20.1
efd8cc9ee8eb919c2ca0f0eebaeb8f39557d8a8a	refs/tags/v0.19.0
82dc6525aa91eb2cbbe5fe591a737d4f03138886	refs/tags/v0.16.0
cc57665e632f7d6b55e5b7950896f1bd26e02e43	refs/tags/v0.15.2
8f135e9f6e354c5ade9f038201f968862f1fb7ef	refs/tags/v0.15.1

vs without --refs:

> git ls-remote --tags --sort -v:refname https://github.com/tree-sitter/tree-sitter-javascript | head -n5
f1e5a09b8d02f8209a68249c93f0ad647b228e6e	refs/tags/v0.20.1^{}
dc2b73b62f162860cc8b55f51a21833ff98867e5	refs/tags/v0.20.1
efd8cc9ee8eb919c2ca0f0eebaeb8f39557d8a8a	refs/tags/v0.19.0
92a561dfa76cb5a4aafba16f819c3c9e1227ce83	refs/tags/v0.16.0^{}
82dc6525aa91eb2cbbe5fe591a737d4f03138886	refs/tags/v0.16.0

Note that f1e5a09b8d02f8209a68249c93f0ad647b228e6e is valid but the reference commit dc2b73b62f162860cc8b55f51a21833ff98867e5 is not...

@smacker smacker merged commit e703811 into smacker:master Dec 15, 2023
4 checks passed
@smacker
Copy link
Owner

smacker commented Dec 15, 2023

great fix! I encountered this problem before but never had time to look into it. Thank you so much!

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.

2 participants