-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add runtime language configuration (#1794) #1866
Conversation
* Add set-language typable command to change the language of current buffer. * Add completer for available language options.
f42320a
to
ab7c1c3
Compare
@@ -537,6 +537,10 @@ impl Loader { | |||
None | |||
} | |||
|
|||
pub fn language_configs(&self) -> impl Iterator<Item = &Arc<LanguageConfiguration>> { |
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 think creating a function like language_config_for_scope
would be more homogenous with the code.
Edit: Ah, you're using fuzzy finding. Never mind.
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.
Yes. 🙂 Though, now that you mentioned it, I could've added a language_config_for_id
functions which could've been useful in the actual command implementation...
helix-term/src/commands/typed.rs
Outdated
|
||
let doc = doc_mut!(cx.editor); | ||
let loader = Some(cx.editor.syn_loader.clone()); | ||
let config = cx.editor.syn_loader.language_configs().find_map(|config| { |
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.
Rather than a custom implementation, set_language2 already takes a scope name and looks up the appropriate language. See the discussion on #1787
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.
Maybe a set_language3
that takes the name of a lang? doing :lang scope.rust
isn't intuitive IMO.
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.
set_language2(format!("scope.{}", 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.
If that's the approach taken, then the fuzzy finding for the languages should be changed to look through the scopes rather than the language names. There are a few langs where the names and scope endings aren't 1-to-1 (e.g. git-diff
v. scope.diff
)
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 didn't use scope
because the prefixes don't seem to follow the source.<lang>
format all the time and the language id doesn't always match with suffix either.
Here's a list of those that don't match with language id:
- protobuf source.proto
- c-sharp source.csharp
- javascript source.js
- typescript source.ts
- html text.html.basic
- latex source.tex
- ocaml-interface source.ocaml.interface
- racket source.rkt
- comment scope.comment
- llvm-mir source.llvm_mir
- llvm-mir-yaml source.yaml
- markdown source.md
- git-commit git.commitmsg
- git-diff source.diff
- git-rebase source.gitrebase
- git-config source.gitconfig
- solidity source.sol
Edit: I agree that adding set_language3
that takes the language id would be nicer, also adding syntax::Loader::language_config_for_language_id()
could be added to not poke into other structs too much.
helix-term/src/commands/typed.rs
Outdated
None | ||
} | ||
}); | ||
doc.set_language(config, loader); |
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.
Similar to #1787 you also need to call refresh_language_server to change language servers. The function also needs to be fixed: it the server changes we need to send a text document close to the old one.
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.
The text document close seems to be already handled in Editor::launch_language_server()
, more specifically from L464:
if let Some(language_server) = language_server {
// only spawn a new lang server if the servers aren't the same
if Some(language_server.id()) != doc.language_server().map(|server| server.id()) {
if let Some(language_server) = doc.language_server() {
tokio::spawn(language_server.text_document_did_close(doc.identifier()));
}
let language_id = doc.language_id().map(ToOwned::to_owned).unwrap_or_default();
// TODO: this now races with on_init code if the init happens too quickly
tokio::spawn(language_server.text_document_did_open(
doc.url().unwrap(),
doc.version(),
doc.text(),
language_id,
));
doc.set_language_server(Some(language_server));
}
}
The only fixing that's needed for Editor::refresh_language_server()
is to check if user already set a language or not, otherwise the detection will overwrite the user set value.
pub fn refresh_language_server(&mut self, doc_id: DocumentId) -> Option<()> {
let doc = self.documents.get_mut(&doc_id)?;
- doc.detect_language(self.syn_loader.clone());
+ // try detection only if language is not already set for this document.
+ if doc.language.is_none() {
+ doc.detect_language(self.syn_loader.clone());
+ }
Self::launch_language_server(&mut self.language_servers, doc)
}
I'm not sure if this check should be part of Document::detect_language()
or not.
* Add language id based config lookup on `syntax::Loader`. * Add `Document::set_language3` to set programming language based on language id. * Update `Editor::refresh_language_server` to try language detection only if language is not already set.
I've played around with using In the last commit I've added a |
* Move document language detection to where the scratch buffer is saved. * Rename Document::set_language3 to Document::set_language_by_language_id.
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.
Just one minor comment
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.
Nice work!
Closes #1794