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

Implement script editor hover hint with description #63908

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

Conversation

Spartan322
Copy link
Contributor

@Spartan322 Spartan322 commented Aug 4, 2022

Supersedes #41502
Closes godotengine/godot-proposals#1393

Right now this is just a rebase and basic conversion of #41502 to master, (including CodeEdit changes) I tried to ensure it has a bare minimum functionality testing it, it does properly display the hints, but some things are as of now missed, and I seem to have not gotten documentation to work properly, it'll stay a draft until I can clean this up a bit.

This is no longer a direct conversion of #41502, there are things I missed in implementing this proposal in full, I'm not entirely confident this is even a totally correct implementation, hence why I left the initial convert commit (despite being somewhat bugged) unchanged for now and the big reason its still a draft, (the initial conversion was still fairly bugged too and sort of re-implemented rich text popup functionality in CodeEdit) however this does implement many more improvements, alongside some few bugs. I committed this to get eyes on it and to improve this beyond what I've been able to do from this point.

Improvements

  • Hover hints appear on the line below the first character of the hovered symbol
  • Supports hover hints for annotations
  • Uses editor settings to enable and modify the hint display
  • Relies upon RichTextLabel inside a PopupPanel to display hint text
    • The hint can't currently be focused but it does use a duplicate of the _add_text_to_rt function from /editor/editor_help.cpp.
  • Local variables should be able to support descriptions
  • All inbuilt classes can display hover hints with descriptions
  • CodeEdit only has a new symbol_hovered(symbol : String, symbol_column_line : Vector2) signal that ScriptTextEditor relies on for hover hints
  • Added get_word_line_column_at_pos(mouse_pos : Vector2) -> Vector2i to TextEdit
    • Duplicates get_word_at_pos but retrieves the line and column of the first character of the word, used by the symbol_hovered signal to abstract the need for retrieving the mouse position.

Known Issues

  • The RichTextLabel may overflow the popup in some cases
  • Short symbols may erroneously be wrapped by the RichTextLabel inside the popup
  • Hovered symbols may feel incorrect since they do not account for the last character, for single character identifiers this can result in a very deceptively tiny region to activate the hint. (this appears to be the fault of get_word_at_pos)
  • Symbols inside a $ may be treated as the type they are named after regardless of whether they're actually based on that type (for example if you named a Popup as PopupMenu, the symbol hint for $PopupMenu will display a hint for PopupMenu)

@akien-mga akien-mga added this to the 4.x milestone Aug 4, 2022
@Spartan322 Spartan322 changed the title Implement TextEdit hover hint with description Implement CodeEdit hover hint with description Aug 4, 2022
@Spartan322 Spartan322 force-pushed the script-editor-hover-description branch from 8207caa to 97c52ea Compare August 7, 2022 01:19
@Spartan322 Spartan322 force-pushed the script-editor-hover-description branch from 97c52ea to c47755a Compare August 20, 2022 22:05
@Spartan322 Spartan322 changed the title Implement CodeEdit hover hint with description Implement ScriptTextEditor hover hint with description Aug 20, 2022
@Spartan322 Spartan322 changed the title Implement ScriptTextEditor hover hint with description Implement script editor hover hint with description Aug 20, 2022
@Spartan322 Spartan322 force-pushed the script-editor-hover-description branch from 005a497 to 08c3599 Compare September 23, 2022 01:34
@Spartan322 Spartan322 force-pushed the script-editor-hover-description branch 6 times, most recently from ad65f6f to 34ef392 Compare March 15, 2023 01:23
@Spartan322 Spartan322 force-pushed the script-editor-hover-description branch from 34ef392 to 39ad7ec Compare May 19, 2023 09:50
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 851bc64), it works. I think this is a good start, and having this feature will improve the script editor's usability a lot.

simplescreenrecorder-2023-07-17_00.08.40.mp4

There are a few usability issues I noticed:

  • Script descriptions don't seem to appear for user-provided properties and methods, even if you restart the editor.

  • Strings that match a function name (without parentheses after the function name) will display the function popup (see the end of the above video).

  • The hover duration seems pretty low – I frequently accidentally make the popup appear when I don't want to. This is annoying, as the popup also blocks mouse input including scrolling (it scrolls the block if possible, instead of the script).

    • In comparison, it takes roughly twice as long for hover tooltips to show up in VS Code.
  • Colors are hard to read on a light editor theme, even after restarting the editor:

simplescreenrecorder-2023-07-17_00.10.28.mp4

These issues need to be addressed somehow before this PR can be merged.

@Spartan322
Copy link
Contributor Author

All right, I'll see what I can do.

ThakeeNathees and others added 2 commits July 17, 2023 11:10
	Replaced with symbol_hovered(symbol, symbol_col_line) signal
Added annotations to hint lookup
Expanded hint lookup
Added SymbolHintHelpBit to display hover hints in ScriptTextEditor
Removed p_lookup parameter from ScriptTextEditor::_validate_symbol
	Moved functionality to _hovered_symbol instead
Added _show_symbol_hint and _hide_symbol_hint to ScriptTextEditor
Added text_editor/hints/hover editor settings
Added TextEdit::get_word_line_column_at_pos
@Spartan322 Spartan322 force-pushed the script-editor-hover-description branch from 39ad7ec to 37d50ce Compare July 17, 2023 15:10
@dalexeev dalexeev mentioned this pull request Sep 25, 2023
75 tasks
@dalexeev
Copy link
Member

@YuriSizov YuriSizov modified the milestones: 4.x, 4.3 Sep 25, 2023
@Spartan322
Copy link
Contributor Author

Yeah I'm aware, I've gotten busy and haven't had as much time to work on this.

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
7 participants