Feat/inlay hints #2525
Feat/inlay hints #2525
Conversation
|
Hi @omprakaash are you working on this? |
|
Hi, Yes. Sorry, progress has been slow for a while due to college. I believe, I am close to putting this up for review by this week. |
There was a problem hiding this comment.
Created a separate lsp_printer. I feel that this pretty similar to the pretty_printer so used an instance of one as a field here. Not sure if this is a good design decision.
|
Hello! Are you currently working on this? |
|
Hey yes. Wanted to get this reviewed before potentially going further. Felt like this was getting too large for a single PR. |
| pretty::{break_, concat, nil, Document, Documentable}, | ||
| }; | ||
|
|
||
| use super::{pretty::Printer, Type, TypeVar}; |
There was a problem hiding this comment.
I think we can just append to a string as inline hints don't need to be wrapped onto multiple lines. Would make it simpler and faster
There was a problem hiding this comment.
Ah gotcha ! Will change this.
There was a problem hiding this comment.
Thank you. I've left a bunch of notes inline.
This is a very large PR and it is proving hard to merge. Would you like it if we split it into a series of smaller and easier PRs? For example, the annotation printer first, then a code action for annotate assignment, then module constant, etc etc.
|
|
||
| #[serde(default = "InlayHintsConfig::default_variable_assignments")] | ||
| pub variable_assignments: bool, | ||
| } |
There was a problem hiding this comment.
Could you document in detail what these do please 🙏
| true | ||
| } | ||
| fn default_variable_assignments() -> bool { | ||
| true |
There was a problem hiding this comment.
I suspect we want this one off by default
There was a problem hiding this comment.
Yeah, was just testing this. Will switch it back to false when complete
| let type_qualifiers = get_type_qualifiers(module); | ||
| let module_qualifiers = get_module_qualifiers(module); |
There was a problem hiding this comment.
These are only used in the code action function so let's move these function calls in there.
| if let Some((AssignName::Variable(v), _)) = &import.as_name { | ||
| let _ = module_qualifiers.insert(import.module.clone(), v.clone()); | ||
| } |
|
|
||
| /// Response handler for the `workspace/configuration` request | ||
| fn configuration_update_received(&mut self, result: Json) { | ||
| if result.is_array() && !result.get(0).is_some_and(|config| config.is_null()) { |
There was a problem hiding this comment.
How come we're not using serde here?
| Type::Named { | ||
| name, args, module, .. | ||
| } => { | ||
| let key = module.clone() + "." + name.clone(); |
There was a problem hiding this comment.
Let's use (&str, &str) as the key to avoid the 3 allocations here.
| // Ignoring module names for in-built gleam types like Int | ||
| name.to_string() |
There was a problem hiding this comment.
This could produce incorrect results when the prelude types have been shadowed locally.
| } => { | ||
| let key = module.clone() + "." + name.clone(); | ||
| let res = self.type_qualifiers.get(&key).unwrap_or(&key).to_string(); | ||
| if args.is_empty() { |
There was a problem hiding this comment.
We're gunna want to have the correct qualifier even when there are args, no?
| position, | ||
| kind: Some(InlayHintKind::TYPE), | ||
| tooltip: None, | ||
| padding_left: text.starts_with(' ').then_some(true), |
| } | ||
| } | ||
|
|
||
| pub fn extract_type_parameters_from_fn(function: &TypedFunction) -> HashMap<u64, TypeAst> { |
There was a problem hiding this comment.
Oh we do this to make a note of all user given type-parameter names anywhere in the function. We call this function before we process a function definition to generate inlay-hints.
eg:
fn equals(a: value, b) { a == b } // In this case b's type would also be annotated as (value)
|
Hey, yes I think it is better if we split this up into separate PRs. What order do you think works best? I think the order you mentioned is good(LSP Printer, variable assignment, module constant etc. ). |
|
Sounds good to me! Whichever is easiest |
|
Closing due to inactivity. Please feel free to reopen! Thank you |
Adds basic support for inlay-hints and annotation insertion for types.