Skip to content
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

Code Editor: Add documentation tooltips #91060

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev commented Apr 23, 2024

@dalexeev dalexeev added this to the 4.x milestone Apr 23, 2024
@dalexeev dalexeev requested review from a team as code owners April 23, 2024 11:55
@KoBeWi

This comment was marked as resolved.

@Atenvardo

This comment was marked as off-topic.

@dalexeev dalexeev force-pushed the code-edit-add-doc-tooltips branch from b94d3fa to 03b4d79 Compare December 7, 2024 21:16
@dalexeev dalexeev requested review from a team as code owners December 7, 2024 21:16
@dalexeev dalexeev force-pushed the code-edit-add-doc-tooltips branch from 03b4d79 to bec6fd0 Compare December 7, 2024 21:20
@adamscott adamscott self-requested a review December 7, 2024 22:25
@dalexeev dalexeev force-pushed the code-edit-add-doc-tooltips branch from bec6fd0 to d4f27de Compare December 11, 2024 09:36
@dalexeev dalexeev requested a review from a team as a code owner December 11, 2024 09:36
@dalexeev dalexeev force-pushed the code-edit-add-doc-tooltips branch from d4f27de to 1a1075c Compare December 11, 2024 09:38
@dalexeev dalexeev force-pushed the code-edit-add-doc-tooltips branch from 1a1075c to 3515e37 Compare December 11, 2024 22:00
@KoBeWi
Copy link
Member

KoBeWi commented Dec 11, 2024

Tested again and it works great now. One major issue is that when moving cursor very slightly the tooltip will disappear, even if you move it over the same symbol. It will open again, but the frequent blinking is bothersome.

Also if you hover a symbol and then move the cursor far away, even outside script editor, the tooltip will still appear.

godot.windows.editor.dev.x86_64_i5Yg4ovIAM.mp4

@dalexeev dalexeev force-pushed the code-edit-add-doc-tooltips branch from 3515e37 to 3e0c867 Compare December 12, 2024 17:31
@dalexeev dalexeev requested a review from a team as a code owner December 12, 2024 17:31
@dalexeev
Copy link
Member Author

Ready for testing/review.

Along the way I noticed/fixed a few bugs in gdscript_editor.cpp, gdscript_docgen.cpp, and script_text_editor.cpp, but there are still some things that make sense to fix separately, as I'm afraid that this might be a significant refactoring (enums, FQCN).

Also if you hover a symbol and then move the cursor far away, even outside script editor, the tooltip will still appear.

Fixed, thanks.

One major issue is that when moving cursor very slightly the tooltip will disappear, even if you move it over the same symbol. It will open again, but the frequent blinking is bothersome.

I understand that Visual Studio Code does it better (the tooltip doesn't disappear until the mouse leaves the symbol), but I'd rather leave it as is for now. This is consistent with the behavior in the inspector and signal dock, and it's also the same as in Kate.

But most importantly, it's technically hard to do. The tooltip checks if the mouse has left the target. This works for simple controls, but not for Tree and CodeEdit, which consist of many visual parts that are not nodes. Also, TextEdit/CodeEdit allows you to get the symbol at a given position as a string, but has no API to get positional information so we can determine whether it was the same symbol or another such identifier on the same line.

I'd suggest leaving non-critical issues for later PRs, as I don't want to further affect unrelated parts of the engine. In my opinion, this is not a very critical issue, I tried to make it so that the tooltip does not interfere with keys, clicks, scrolling and text selection.

Another usability issue is that TextEdit finds a symbol outside the line:

But the same happens if you hold down Ctrl:

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2024

Hovering over variable definition does not show a tooltip. It could be useful when using inference, so e.g. you can hover var something := method() and know the type of something at the source, without having to find its usage.

This is consistent with the behavior in the inspector and signal dock, and it's also the same as in Kate.

For some reason I find it more difficult to see tooltips in code editor than in the inspector. Maybe the minimum movement is too small?

@dalexeev dalexeev force-pushed the code-edit-add-doc-tooltips branch from 3e0c867 to 7ba0c58 Compare December 12, 2024 21:25
@dalexeev
Copy link
Member Author

Hovering over variable definition does not show a tooltip. It could be useful when using inference, so e.g. you can hover var something := method() and know the type of something at the source, without having to find its usage.

Looks like it's not implemented in the parser:

COMPLETION_BUILT_IN_TYPE_CONSTANT_OR_STATIC_METHOD, // Constants inside a built-in type (e.g. Color.BLUE) or static methods (e.g. Color.html).
COMPLETION_CALL_ARGUMENTS, // Complete with nodes, input actions, enum values (or usual expressions).
// TODO: COMPLETION_DECLARATION, // Potential declaration (var, const, func).
COMPLETION_GET_NODE, // Get node with $ notation.
COMPLETION_IDENTIFIER, // List available identifiers in scope.

When I hover over the variable declaration identifier I get {type = GDScriptParser::COMPLETION_NONE, current_class = 0x0, current_function = 0x0, current_suite = 0x0, current_line = -1, current_argument = -1, builtin_type = Variant::VARIANT_MAX, node = 0x0, base = 0x0, parser = 0x0}.

This and some other issues with incorrect hints are material for future PRs.

For some reason I find it more difficult to see tooltips in code editor than in the inspector. Maybe the minimum movement is too small?

I can't reproduce it on Linux. I encountered this during development, but at some point the bug disappeared.

@@ -3753,6 +3789,13 @@ CodeEdit::CodeEdit() {
set_gutter_custom_draw(gutter_idx, callable_mp(this, &CodeEdit::_fold_gutter_draw_callback));
gutter_idx++;

/* Symbol tooltip */
symbol_tooltip_timer = memnew(Timer);
symbol_tooltip_timer->set_wait_time(GLOBAL_GET("gui/timers/tooltip_delay_sec"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to update it with settings_changed signal.

@@ -166,6 +188,39 @@ static String _contextualize_class_specifier(const String &p_class_specifier, co
return p_class_specifier.substr(p_edited_class.length() + 1);
}

/// EditorHelpBit ///

// TODO: this is sometimes used directly as doc->something, other times as EditorHelp::get_doc_data(), which is thread-safe.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: this is sometimes used directly as doc->something, other times as EditorHelp::get_doc_data(), which is thread-safe.
// TODO: This is sometimes used directly as doc->something, other times as EditorHelp::get_doc_data(), which is thread-safe.

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2024

Looks like the description can't be determined in complex code:
image
Even though Ctrl+Click jumps to the member and it has a description.

I assume this is related to #95821?
EDIT:
Yeah, I saved the script and it works.
EDIT2:
Interestingly, the tooltip also misses method's signature, but it's able to infer correct variable type from the method's return value.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base functionality works fine. I agree it can be improved later if needed.
I checked the code of editor and gui in the second commit and also looks ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement script editor description hint on hover a symbol/word
5 participants