-
Notifications
You must be signed in to change notification settings - Fork 600
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
lsp: Offer to populate empty documents #4767
base: master
Are you sure you want to change the base?
Conversation
Of course this only shows while the file does not contain anything but whitespace:-) |
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. I like it.
I think only one option is enough though.
Probably the Hello World like so:
import { AboutSlint, VerticalBox } from "std-widgets.slint";
export component MainWindow inherits Window {
VerticalBox {
alignment: start;
Text { text: "Hello World!"; }
AboutSlint { preferred-height: 150px; }
}
}
Make it a window by default. But keep it as short as possible while still have something somehow visual.
Did you try if it worked in a slint!{}
macro?
tools/lsp/language.rs
Outdated
if let Some(current_version) = version { | ||
if current_version != source_version { | ||
return Err( | ||
"Document version mismatch. Please refresh your command data".into() |
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 is the kind of error that is seen by the user when something goes wrong in synchronization. But users will have no idea how to "refresh their command data".
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.
Well, when that happens the LSP stops anyway :-)
I'll just return "OK" and be done with that...
tools/lsp/language.rs
Outdated
if let Some(range) = util::map_node(node) { | ||
let has_non_ws_token = node | ||
.children_with_tokens() | ||
.any(|nt| nt.kind() != SyntaxKind::Whitespace && nt.kind() != SyntaxKind::Eof); |
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.
Should we also accept a comment? (eg license header comments)
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.
Did you try if it worked in a slint!{} macro?
Yes and that fails hard:-/
It replaces the entire rust code.
We need to remember which area of the document we are actually touching... otherwise this just can not work. |
The latest set of commits add an ASCII start-of-text and end-of-text byte around the area covered by It then limits the actions to the range between these markers (if present). That way it works with the slint! macro BUT there is a bug still: When you remove the slint macro, the LSP will not update its document cache accordingly and will insist that the code lense needs to continue to exist. That needs fixing :-/ |
Add `Start-of-text` ASCII value in the place of the opening brace of the slint macro and `End-of-text` in place of the closing bracket. ASCII chars are one byte, so they will always work :-)
So when snippets do not work: Let's try something else to get started from scratch:-)