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

Improve GDScript autocompletion for methods #99102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lazy-Rabbit-2001
Copy link

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 commented Nov 12, 2024

Implements the part 1 of godotengine/godot-proposals#11079
Check the link for details about the reason why this pr is posted.

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 changed the title Improve GDScript autocompletion Improve GDScript autocompletion for methods and signals Nov 12, 2024
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 marked this pull request as ready for review November 12, 2024 06:18
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 requested a review from a team as a code owner November 12, 2024 06:18
@dalexeev dalexeev added this to the 4.x milestone Nov 12, 2024
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 changed the title Improve GDScript autocompletion for methods and signals Improve GDScript autocompletion for methods Nov 12, 2024
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 requested a review from a team as a code owner November 12, 2024 13:14
@Lazy-Rabbit-2001
Copy link
Author

Kinda off-topic, but it surprised me that the back braces will not get overlapped. In previous versions, trying completing your code inside a couple of round brackets will remove the back brace if the completion is about a method or a keyword with (), like this:

print(is_inside_tree() # <- Here the back brace of `is_inside_tree()` is missing, or overlapping with the outer one.

I'm not sure if it is this pr or some other pr in this version that fixed this bug, or maybe it's because I haven't tested edge situation yet. But it seems to be a good sign.

@Calinou
Copy link
Member

Calinou commented Nov 13, 2024

Kinda off-topic, but it surprised me that the back braces will not get overlapped.

It depends on whether you accept the suggestion with Tab or Enter. See #90723 which changes this behavior to be more predictable.

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, it works as expected. Code looks good to me.

image

This is a nice usability improvement 🙂

@AdriaandeJongh
Copy link
Contributor

Considering the autocomplete popup uses a monospaced font, and the three dots inside function parentheses take up a considerable width, may I suggest using this character instead?

@HolonProduction
Copy link
Member

This is indeed a nice improvement. I think the usage of works better:

image

Looks good to me on code and functionality side. You still need to squash the commits before this can get merged.

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 force-pushed the autocompletion-optimization branch 2 times, most recently from 7c6dd8e to 9c9a746 Compare November 22, 2024 03:10
@HolonProduction
Copy link
Member

Ok, now it's squashed, but could we get a meaningful commit name? E.g. just the title of the PR. If you already squashed you can use amend for it:

git commit --amend -m "Improve GDScript autocompletion for methods"
git push -f

modules/gdscript/gdscript_editor.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_editor.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_editor.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_editor.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_editor.cpp Outdated Show resolved Hide resolved
@Chaosus Chaosus modified the milestones: 4.x, 4.4 Nov 22, 2024
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.

7 participants