Skip to content

fix: ensure vendored grammar is clean before fetch#14817

Merged
the-mikedavis merged 1 commit intohelix-editor:masterfrom
RoloEdits:fix-grammar-fetch
Nov 24, 2025
Merged

fix: ensure vendored grammar is clean before fetch#14817
the-mikedavis merged 1 commit intohelix-editor:masterfrom
RoloEdits:fix-grammar-fetch

Conversation

@RoloEdits
Copy link
Copy Markdown
Contributor

@RoloEdits RoloEdits commented Nov 17, 2025

The issue came about when trying to do #14814. For some reason the state of the vendored repo was dirty and prevented any fetch:

Fetching 273 grammars
  271 up to date git grammars
  Failure 1/2: markdown_inline Git command failed.
  Stdout:
  Stderr: error: Your local changes to the following files would be overwritten by checkout:
        contrib/screenshot.png
  Please commit your changes or stash them before you switch branches.
  Aborting

  Failure 2/2: markdown Git command failed.
  Stdout:
  Stderr: error: Your local changes to the following files would be overwritten by checkout:
        contrib/screenshot.png
  Please commit your changes or stash them before you switch branches.
  Aborting

Initially I tried to add

git(&grammar_dir, ["reset", "--hard"])?;
git(&grammar_dir, ["clean", "-fdx"])?;

but this only made the CI go from all jobs erroring, to only the tests erroring, with msrv, lint, and doc jobs passing. I also tried to --force on fetch, but the issue remained.

For resolving this, I just went about it the simplest method of just removing the directory before initializing it again, making a clean state to fetch from.

Looking at the diff, github did a pretty horrible job, but basically I reduced nesting in fetch_grammar by utilizing early returns, and encapsulated vendored repo operations with a new VendoredGrammar with various methods that do the same thing as before, now adding a step before fetching to reinitialize the clean repo state.

@RoloEdits
Copy link
Copy Markdown
Contributor Author

Forgot to mention, but #14814 is based off of this, and that now seems to no longer have any issues.

@RoloEdits RoloEdits force-pushed the fix-grammar-fetch branch 2 times, most recently from f89afd3 to 7920372 Compare November 18, 2025 05:25
@RoloEdits
Copy link
Copy Markdown
Contributor Author

CI fail is due to the spurious windows one we have been having for a write test.

Copy link
Copy Markdown
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

I think we saw this kind of error in the past for one grammar, but it only affected Windows. Removing the directory and checking out from scratch seems like the most straightforward thing to do here 👍

@the-mikedavis the-mikedavis merged commit d2fccd5 into helix-editor:master Nov 24, 2025
6 of 7 checks passed
a3lem pushed a commit to a3lem/helix that referenced this pull request Jan 11, 2026
Eucladia pushed a commit to Eucladia/helix that referenced this pull request Jan 20, 2026
fryeb pushed a commit to fryeb/helix that referenced this pull request Jan 23, 2026
@RoloEdits RoloEdits deleted the fix-grammar-fetch branch March 28, 2026 05:16
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