-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Dynamically load grammar libraries at runtime #432
Conversation
50a6c9f
to
2993d9c
Compare
2993d9c
to
ba5981c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seemed like a method for plugin since we are doing libloading.
}; | ||
std::mem::forget(library); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about simply forgetting the library, maybe we can return a wrapper around Language
and Library
so their lifetime is tied (and Library is unloaded when the Language is not used anymore).
Just to be sure, is get_language
called exactly once for each language?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied this verbatim from tree-sitter (though they admittedly use it more short lived). Or LanguageConfig
& HighlightConfig
are static, they only get initialized once (wrapped in a OnceCell
) then live indefinitely in the registry and are never dropped. So I think for the time being this forget
is fine (it implies the grammar lives as &'static)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!
Windows places temporary files in the current dir, so compiling in parallel caused conflicts.
7fa9eea
to
605ce98
Compare
605ce98
to
3d0f190
Compare
let mut result = String::with_capacity(name.len()); | ||
for c in name.chars() { | ||
if c == '-' { | ||
result.push('_'); | ||
} else { | ||
result.push(c); | ||
} | ||
}; | ||
} | ||
result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just name.replace('-', "_")
?
Motivated by some of the findings in #330 I went ahead and attempted to split out grammars.
The set of languages is no longer static, this way the user will be able to self-compile additional grammars and provide them under
runtime/grammars
. Languages can now be declared without a matching grammar too, which should unblock #205.I think it would be cool to expose some of the
build.rs
code as an optional feature under thehx
CLI, so that the user would be able to easily point to a cloned tree sitter grammar and compile it into their runtime dir.Additionally, the grammar objects are now persisted and we only recompile when necessary (if the grammar sources changed), which should massively speed up
cargo clean && cargo build
.With these changes, release build of
hx
is now7.8M
after stripping.