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

Merged

Conversation

Lazy-Rabbit-2001
Copy link
Contributor

@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 force-pushed the autocompletion-optimization branch from 50cbe28 to 17d69b0 Compare November 12, 2024 04:49
@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
Contributor 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

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 force-pushed the autocompletion-optimization branch from 9c9a746 to 425976d Compare November 22, 2024 10:16
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 force-pushed the autocompletion-optimization branch from 425976d to ae853e1 Compare November 22, 2024 10:24
@Chaosus Chaosus modified the milestones: 4.x, 4.4 Nov 22, 2024
@Repiteo Repiteo merged commit 757a1d3 into godotengine:master Nov 22, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 22, 2024

Thanks! Congratulations on your first merged contribution! 🎉

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 deleted the autocompletion-optimization branch November 23, 2024 02:39
@Meorge
Copy link
Contributor

Meorge commented Nov 26, 2024

I'm sorry to be a bit of a downer on this, but at least would there be a way to change this behavior depending on whether or not Godot's internal editor is receiving the suggestions versus an external IDE? The new behavior is inconsistent with other languages in VS Code, where the parentheses are never given after brackets in the suggestion list or the actual completed code.

Currently, for GDScript, starting to type out print:
CleanShot 2024-11-25 at 22 16 04@2x
Immediately after hitting Enter/Return:
CleanShot 2024-11-25 at 22 16 19@2x

By contrast, here are some other popular languages in VS Code:

For Python, starting to type out print:
CleanShot 2024-11-25 at 22 08 19@2x
Immediately after hitting Enter/Return:
CleanShot 2024-11-25 at 22 08 41@2x

For TypeScript, starting to type out console.log:
CleanShot 2024-11-25 at 22 09 35@2x
Immediately after hitting Enter/Return:
CleanShot 2024-11-25 at 22 09 43@2x

For C/C++, starting to type out print_line:
CleanShot 2024-11-25 at 22 13 33@2x
Immediately after hitting Enter/Return:
CleanShot 2024-11-25 at 22 13 43@2x

It seems the standard is to not include parentheses in the suggestions for function/method names, and to not insert them once the user accepts the suggestion. Perhaps it makes more sense for Godot's built-in editor, but for external ones such as VS Code it looks to be different. IMO it'd make sense to either change the autocomplete behavior depending on the recipient (built-in editor vs external editor), or change the built-in editor's behavior so that it is more consistent with other language servers.

@HolonProduction
Copy link
Member

I think only referencing language servers developed by Microsoft might be a bit biased (I suppose you are using the Microsoft c++ language server).

Here are some counter examples:

clangd C++ language server:
Screenshot From 2024-11-26 11-07-13
Screenshot From 2024-11-26 11-07-23

Dart integration
image
image

Java (the oracle plugin)
image
image

@Meorge
Copy link
Contributor

Meorge commented Nov 26, 2024

Thanks for providing the counterexamples! You're right that they were all Microsoft-developed servers, as far as I know - my thought was that since Microsoft originally developed the specification, if there was a standard for how/when to do things like insert parentheses, the Microsoft servers would be the most likely to follow it to a T.

The documentation for VS Code's Rust support on their website features rust-analyzer, which also shows (...) after function/method names 🙂

Given that these other big language servers do it as well, I agree having the GDScript server do it is all good. 😄

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.

9 participants