Refactor the injection marker type #12561
Open
+40
−22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This splits the `InjectionLanguageMarker::Name` into two: one that preforms the previous behavior (using the language configurations' `injection_regex` fields and performing a match) and a new variant that looks up directly by `language_id` with equality. The old variant is used when capturing the injection language like we do in the markdown queries for codefences. That captured text is part of the document being highlighted so we might need a regex to recognize a language like JavaScript as either "js" or "javascript". But the text passed in the `(#set! injection.language "name")` property can be looked up directly. This property is in the query code so there's no need to be flexible in what we accept: we can require that the `(#set! injection.language ..)` properties refer to languages by their configured ID. This should save a noticeable amount of work for the common case of injections: `(#set! injection.language)` is used much more often than `@injection.language`.
The `Name` variant's inner type can be switched to `RopeSlice` since the parent commit removed the usage of `&str`. In doing this we need to switch from a regular `Regex` to a `rope::Regex`, which is mostly a matter of renaming the type. The `Filename` and `Shebang` variants can also switch to `RopeSlice` which avoids allocations in cases where the text doesn't reside on different chunks of the rope. Previously `Filename`'s `Cow` was always the owned variant because of the conversion to a `PathBuf`.
|
Would this close #3072? |
I tried it and had the same result, it feels about twice as fast as before, but still a bit laggy. Great work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a rewrite of the
LanguageInjectionMarkertype with two purposes:LanguageIdvariant that can be used to look up languages bylanguage_id. This is used instead ofNamefor(#set! injection.language "lang").Name,FilenameandShebangvariants can all useRopeSliceas their inner type and avoid allocations in the common case.The first change should be the more impactful one:
(#set! injection.language "language-id")is far more common than capturing@injection.language, which is used for things like markdown code fences. Previously we treated both as theNamevariant meaning that we looked the languages up by the longest matchinginjection_regex. Instead we can lookup by checking equality with thelanguage_idfield which is much cheaper. There's really no need to be flexible in what we accept for the(#set! injection.language ..)version. For me locally this change noticeably reduces editing lag on a large Markdown file like theCHANGELOG.md.