Skip to content

Add support for hidden_file_extensions key.#419

Merged
Enselic merged 1 commit into
trishume:masterfrom
sourcegraph:vg/upstream/hidden-file-extensions
Mar 14, 2022
Merged

Add support for hidden_file_extensions key.#419
Enselic merged 1 commit into
trishume:masterfrom
sourcegraph:vg/upstream/hidden-file-extensions

Conversation

@varungandhi-src

@varungandhi-src varungandhi-src commented Jan 16, 2022

Copy link
Copy Markdown
Contributor

This was introduced in Sublime Text 4.

Addresses one part of #323.

As I understand it, it doesn't really matter if a file extension is "hidden" or not from the POV of syntax highlighting.

@varungandhi-src

Copy link
Copy Markdown
Contributor Author

Is anyone actively working on fixing 323/adding support for the other new features? I didn't see any active PRs related to it, and no one had commented on the issue, but I figured I'd ask first.

@keith-hall

Copy link
Copy Markdown
Collaborator

Is anyone actively working on fixing 323/adding support for the other new features?

Not that I know of, no. Iirc I wrote a comment asking for advice on how the API might look like, but had no responses. I did have a quick look at implementing support for popping multiple contexts at once but it wasn't trivial with the current design.

@trishume trishume left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't think anyone else is. However note that the real trickiness will be the backtracking support, which I believe will require a complete API overhaul, since currently you can parse in a streaming way since no line can affect a previous line, whereas with backtracking that's no longer true.

I think it might be reasonable to support everything except backtracking though, since there'll be lots of syntaxes which might use some new features but not backtracking.

@trishume

Copy link
Copy Markdown
Owner

Oh, see the minimum supported rust version CI error, you just need to borrow the array you're iterating.

@varungandhi-src varungandhi-src force-pushed the vg/upstream/hidden-file-extensions branch from 7cce8b4 to 725cdf4 Compare January 17, 2022 04:26
@varungandhi-src

Copy link
Copy Markdown
Contributor Author

CI error should be fixed with the latest push.

As for backtracking, thanks for bringing that up. I'd only seen issue 323 and not issue 271, so I hadn't realized that was another potential pain point before bumping to ST4. It looks like a bunch of grammars are already using the backtracking-related features:

Batch File/Batch File.sublime-syntax
C#/C#.sublime-syntax
Git Formats/Git Commit.sublime-syntax
Java/Java.sublime-syntax
JavaScript/JSX.sublime-syntax
JavaScript/JavaScript.sublime-syntax
JavaScript/TSX.sublime-syntax
JavaScript/TypeScript.sublime-syntax
Lua/Lua.sublime-syntax
Markdown/Markdown.sublime-syntax
PHP/PHP Source.sublime-syntax
Python/Python.sublime-syntax
SQL/SQL.sublime-syntax

Apart from backtracking, it sounds like adding support for the 4075-specific features, such as syntax inheritance, is not as complicated and shouldn't affect the public API -- is that a fair assessment?

@jrappen

jrappen commented Jan 17, 2022

Copy link
Copy Markdown
Contributor

... grammars which are using branch: ...

also add these pending re-writes from PRs:

  • Haskell
  • JSON

@varungandhi-src

Copy link
Copy Markdown
Contributor Author

Could someone please kick-off the PR testing again? The clippy issue should be fixed, and I'm seeing a "First-time contributors need a maintainer to approve running workflows" message.

@trishume

Copy link
Copy Markdown
Owner

Huh I tried re-running it and the clippy issue happened again. Did you already look into it? Looks like the problem is that the time crate updated past our MSRV, which of course isn't a problem with this PR. Perhaps we need to either bump our MSRV or constrain an older version of the time crate.

@Enselic

Enselic commented Jan 23, 2022

Copy link
Copy Markdown
Collaborator

I created a PR to bump MSRV along with a motivation of why I believe that is preferred over putting an upper limit on time: #422

@varungandhi-src

Copy link
Copy Markdown
Contributor Author

Thanks @Enselic, we can retest and land this PR after your PR lands.

@Enselic

Enselic commented Mar 14, 2022

Copy link
Copy Markdown
Collaborator

@varungandhi-src Could you merge origin/master into this branch so CI becomes green please?

This was introduced in Sublime Text 4.
@varungandhi-src varungandhi-src force-pushed the vg/upstream/hidden-file-extensions branch from 725cdf4 to e54eef3 Compare March 14, 2022 17:00
@varungandhi-src

varungandhi-src commented Mar 14, 2022

Copy link
Copy Markdown
Contributor Author

@Enselic rebased and force-pushed; sorry I hadn't realized that PR testing was running pre-merge instead of after locally merging into master.

@Enselic

Enselic commented Mar 14, 2022

Copy link
Copy Markdown
Collaborator

Since this has been Approved by both Tristan and Keith I will merge this now that CI is green. I took a quick look and the code looked good to me too.

I will add an entry for this PR to CHANGELOG at a later point as part of #409

@Enselic Enselic merged commit 6b211f9 into trishume:master Mar 14, 2022
@varungandhi-src varungandhi-src deleted the vg/upstream/hidden-file-extensions branch March 14, 2022 19:40
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.

5 participants