-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
GDScript Symbol Tooltip #80044
GDScript Symbol Tooltip #80044
Conversation
… over. Needs more improvement.
… of a symbol/word in a line.
…itioned appropriately even in the popout window.
Co-authored-by: A Thousand Ships <[email protected]>
Annotations have corresponding docs, so this doesn't seem necessary.
I'd suggest using the tooltip delay in the editor settings (I think?) for a start. We could have a dedicated setting in the future if it's desired imo |
The tooltip delay is actually a project setting (and it affects the editor in the currently edited project). See #35806. |
…p_delay_sec' global setting.
Thank you for catching that! I accidentally wrote 'annotation' here when I meant to put 'comment'... 🙃 so I've updated that task accordingly.
Thank you for the feedback and info. I planned to tackle making the delay configurable in the "Add configurable settings." step of the planned features for this PR, but since there is already a setting and it is easy to fetch it I've added it to the SymbolTooltip constructor. This can be updated to fetch from EditorSettings once that issue/PR is resolved. I will note that I'm aware of the existing |
…rser list of DocumentSymbol members.
…ditor-symbols-tooltip-2 # Conflicts: # editor/symbol_tooltip.cpp
My most recent pushes added tooltip support for:
This is still much work to be done, but this is a great step forward - and the Some things that I'm currently contemplating:
|
The implementation seems to be overly complicated for what we need from this feature. There are two distinct systems that need to exist:
For symbols which are documented (either natively, or via the comments) DocData should be able to provide the information needed. For symbols which aren't, some fallback logic must be present to generate a mock tooltip. So I would expect 3 sets of changes, one to the script editor, one to the editor help, and one to the doc data. I don't particularly see the need for a dedicated class with so much logic in it. Granted, this PR is very much WIP and has a ton of code that should be removed or reworked, so perhaps it could be clearer why this approach was chosen once you finish your work. But at this point I'm skeptical this is going into the right direction. We shouldn't need to hook into language parsers and LSP data structures to achieve the functionality needed here. In fact, none of this should be dependent on individual scripting languages. It should go through public data or through the script server as an abstraction layer. The feature itself is very much desired. I'd appreciate if you and @Spartan322 could cooperate on finding the most appropriate solution. |
@YuriSizov, first off, thanks for taking the time to review and share your thoughts; it's genuinely appreciated. I'll dive a bit into the "why" behind my approach to give you a clearer picture. My Starting Point & Approach:
Current Implementation:
Collaborating and Making Things Happen:
My hope is that even if parts of my approach don't make the final cut, they can still serve as a stepping stone to a refined solution. Again, big thanks for the feedback. I want this feature to be successful and welcome constructive criticism like yours. |
By public data I mean mostly I would advise you start solving this part of the problem by making a panel anywhere in the editor and figuring out a way to fetch the documentation for an arbitrary member of an arbitrary class. Say, pick
That's the second part of the problem. You need the script editor to give you some identifier for the hovered member. I can't tell you exactly what that identifier might look like. Can be some sort of structure or a string that uniquely identifies a property or a method. You can look at other features that hook into such stuff, like syntax highlighter, code completion, and navigation to definition/documentation triggered by ctrl-clicking on it. All of these should at least partially solve the same problem. Maybe @Paulb23 would have some pointers. When you have solved both parts of this problem, it's only a matter of tying them together. Don't worry about the looks and user-facing functionality. For example you can just add a panel somewhere in the script editor and display the details there without bothering with hover behavior, styling, or sizing. Make it functional first. The original PR implemented several years ago didn't have any styling applied to it whatsoever, for instance. |
@jwmcgettigan Try starting with godot/core/object/script_language.h Lines 326 to 335 in 7e67b49
Also see |
|
…ange is within another.
# Conflicts: # modules/gdscript/language_server/gdscript_extend_parser.cpp # modules/gdscript/language_server/godot_lsp.h
…addition of a 'parent' pointer on each symbol.
# Conflicts: # editor/editor_help.h
You're insane for this by the way. Would be amazing to have for 4.3 if you are around to continue this. |
# Conflicts: # modules/gdscript/language_server/gdscript_extend_parser.cpp
I unfortunately haven't had the bandwidth to continue working on this and don't foresee having bandwidth anytime soon. I left off struggling to:
In retrospect, some mistakes I feel I made were:
I think that going forward, ideally:
Some final notes:
I look forward to the day this feature becomes a reality and hope my draft PR can provide a helpful starting point for someone else to take it further! |
|
Planned Features (WIP - subject to change)
Create a basic tooltip component.
Refine hover logic.
Establish tooltip content.
DocumentSymbol
members for eachenum
value.Expand upon tooltip content.
CompletionContext
struct).enum
. For example, add a way for the user to see all of the enum values within the tooltip. The current approach is not conducive to including those values. (Look into how other IDEs do it)var person = Person.new()
)@GDScript
and@GlobalScope
classes.@GDScript
and@GlobalScope
classes.DocumentSymbol
members for each usage of a variable. (e.g. As parameter or within function.)DocumentSymbol
property that contains the context.DocumentSymbol
property that contains the parent member.self
should provide info for the class it references.Parse and display documentation.
Incorporate user code documentation.
[param name]
.Implement link behaviors.
Implement context menu.
Design refinements.
Add error and warning messages.
Add configurable settings.
Create class documentation
docs/classes/SymbolTooltip.xml
.Create unit tests
Known Issues
NOTE: I will keep fixed issues here for the SEO if anyone runs into a similar issue in the future.
PopupPanel
to solve these issues...)_init()
functions return the class rather thanvoid
within theDocumentSymbol.reduced_detail
property.Issues that I could use some help with
TextEdit
for the tooltip header to have syntax highlighting. This may "work" for the MVP, but we should instead implement syntax highlighting on a more appropriate component likeRichTextLabel
.Stretch Goals/Features (Features I'm unlikely to implement but may be worth pursuing by anyone interested)
Implement tooltip resizing.
Implement nested tooltips.
Optional 'on hover' or 'context menu option'
Handle conflicts between built-in and local
Show on auto completions as a sub popup
Notes
Latest Demo GIF (09/08/2023)