-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Support for inlayHints user preferences in language server #60326
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
Comments
I like this idea. We'd need to decide exactly what settings to provide.. If we copy TypeScript (but exclude
Two questions I have:
|
Should we just add a value between
Do we want this to be a fuzzy match? For example, what if the type is
Semantically they're property declarations, so that would be my recommendation.
Or just generalize the two into a single option that controls any location in which type information might be displayed? What about return types, such as a return type of For that matter, should |
I wondered about that too (this is how TS does it and I was copying them for some consistency). I don't know if they had the If we did this, what should it be called, and should we convert the other bools to be an enum for this? (
My feeling is we should be fuzzy only for things like casing/underscores to avoid hiding any potentially important context?
I think for semantic tokens we map them as "instance variable declarations" or something (although references to them are properties because they are references to the getter/setter), but I'm not sure if the logic for that is sound. The reason I wondered about variables is that anything we put together in a category can only be togged together, and I wonder if a class field is something more likely to want to be toggled with variables (since they're of a similar form) compared to getter/setters. (Ofc, we could give them their own category and avoid choosing).
hmm, I think I prefer this to the idea above. I feel like if you've chosen not to include "duplicate" labels, probably you'd want that everywhere?
Good question. I copied this from TS and didn't really question it, but I wonder if there's much value in separating properties at all (eg. getters could use Looking at the Haxe list posted above, that does also seem much simpler than TS with just a few bools:
So maybe something like:
|
The reason this is hard is that it isn't possible (in general) to determine what context, if any, might be sufficient. I assume we'd consider a name to match as long as the type name is somewhere in the name, such as a variable named I know that Erik has done some work to define 'obvious' for types. I wonder whether that might be another factor to take into account. Do we want to inline the type of a variable if the type is obvious based on the initialization expression? For example, consider To be clear, I'm not opposed to making the feature be configurable. I just don't want it to get out of hand. I like the second list of possible options more than the first. |
For some reason I only thought about this the other way around. But I think you're right that the type is probably redundant in this case. I'm less sure about the other way (
I wonder if we should split names into "segments" (where there are capitals, underscores, start/end of identifier, etc.) and then do only complete matches on those? (eg.
I don't know much about this, but if we have an existing concept for this and we could reuse it here, I think that could be nice (and having its own option makes sense). Perhaps it's something to look at separately though - perhaps with a little configurability (as noted above) the feature would be more useful and we'd get better feedback on additional tweaks like that. |
I find the lack of a good definition of what we mean by "matching" to be concerning. I'd like to hold off on that particular preference until we have a more concrete definition of what it does, but the test of the preferences seem reasonable to me.
For more context, see #58773. "Obvious" isn't the same for everyone, but the concept is, I think, well defined for the lints. |
I really don’t understand why this seemingly gets turned into a PhD dissertation. Couldn’t this just be done similarly to how the TypeScript language server handles it - just without the settings that don’t make sense for Dart? Holding off for some “concrete” definitions basically feels like this will never be solved. Is there a dedicated, trackable task whose goal is to figure out these ominous definitions? Sorry if this comes off as rude, that’s not my intention - I’m just genuinely confused why this has to be so complicated. |
I can only speak for myself, but in my experience having a good design leads to a better UX and a more maintainable implementation. As for the approach, I think we are, in fact, talking about doing something similar to how the TypeScript server handles it and were discussing what does and doesn't make sense for Dart. And the only part of this that I wanted to hold off on is the implementation of any kind of But as for why the conversation stopped without reaching a conclusion, it's because this isn't currently considered to be a high priority for our team. We need to focus on the more critical issues, which at the moment includes some significant performance concerns. |
Sorry if it seemed like progress wasn't being made here. My feelings match Brian's - it's difficult to change things like settings once they're shipped, so spending a little time up-front can save effort (or avoid a less-ideal design when changing things) later. I also thought it would be useful to get feedback/opinions (including yours) on what was proposed before starting something that might not solve the original request. I do think this would be a great change for inlay hints because I still find them slightly noisy myself (and therefore use them less than I think I would if this was implemented), but there are also many other ideas (and fixes) competing for resources that this needs to be weighed up against. |
Please support user configuration of the inlayHints support for the Dart language server similar to the JS/TS language server or the Haxe Language Servers .
This allows end-users to configure for which elements they want inlay hints to be displayed.
This seems currently not possible according https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/tool/lsp_spec/README.md
The text was updated successfully, but these errors were encountered: