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

Migrate remaining features into CodeEdit #50122

Merged

Conversation

Paulb23
Copy link
Member

@Paulb23 Paulb23 commented Jul 3, 2021

Continuation of #31739


Apologies for the larger PR was going to limit this to brace completion, but the remaining features seemed small enough to bundle in here.

After this PR, I consider CodeEdit good enough for 4.0 and beyond. Next steps will be cleaning up TextEdit and prepping the API similarly, before bug fixes / optimisation. After this point we can consider #31739 done and can create smaller tasks for the remaining parts, as they will not have an affect on the outward facing API.

The only remaining "code like" feature in TextEdit is highlighting all occurrences of the selected text. I did not consider this a strong enough feature to consider migrating to CodeEdit, though am happy to do so if others think this should be the case.


Firstly, this PR migrates auto brace completion. This has been made to use strings rather then single chars to allow handling of larger symbols such as gdscript multiline comments / strings. And has exposed to the API. A new overridable method handle_unicode_input has been added, which is called when the user types a char. This way the child class does not need to check all shortcuts before handling the input.

For Brace matching, only the API has been migrated and exposed, until drawing can be somewhat controlled in CodeEdit. As such could be classed as "incomplete", in the sense that only hardcoded chars are highlighted and not everything added in auto brace completion.

Secondly, line length guidelines have been moved and exposed. Rather then a soft and hard limit it now takes an array of columns. The first entry is a "hard" limit, the following entries are a "soft" limit and drawn at 50% opacity.

The last feature is symbol lookup, this has been reconfigured and exposed to the API. At the moment, underline drawing is still handled by TextEdit, however everything else is in CodeEdit

Finally, cleaned up the inspector a little by moving gutters into a sub category and changing a few default values. On top of this finished up all the remaining CodeEdit docs.


closes #41361

@Paulb23 Paulb23 added this to the 4.0 milestone Jul 3, 2021
@Paulb23 Paulb23 requested review from a team as code owners July 3, 2021 11:43
@Paulb23 Paulb23 force-pushed the code_edit_auto_brace_completion branch from 2b7d431 to 5e0b50e Compare July 11, 2021 14:57
doc/classes/TextEdit.xml Outdated Show resolved Hide resolved
scene/gui/code_edit.cpp Outdated Show resolved Hide resolved
Comment on lines +233 to +234
String symbol_lookup_new_word = "";
String symbol_lookup_word = "";
Copy link
Member

@KoBeWi KoBeWi Jul 29, 2021

Choose a reason for hiding this comment

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

You don't need to initialize Strings. It's done automatically (unlike primitive types like int or pointers).

Copy link
Member

Choose a reason for hiding this comment

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

I missed this, but that can be done as a follow-up (maybe a regex search as there might be more unneeded String inits in the codebase).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah there's a ton of them:

$ rg 'String .* = "";' | wc -l
106

@KoBeWi
Copy link
Member

KoBeWi commented Jul 29, 2021

I looked over the code and it's mostly ok. Except maybe comments, we usually use // Double slash comments. instead of /*This thing.*/, especially for single-line comments. The old comments using /**/ should be updated too probably.

Also would be nice to have the PR rebased, so it can be tested more easily.

@Paulb23 Paulb23 force-pushed the code_edit_auto_brace_completion branch from 5e0b50e to 481c1d3 Compare August 1, 2021 11:06
@Paulb23 Paulb23 force-pushed the code_edit_auto_brace_completion branch from 481c1d3 to 809a32c Compare August 1, 2021 11:34
@KoBeWi
Copy link
Member

KoBeWi commented Aug 2, 2021

Hover underline doesn't seem to work:
phMXjR4kVn

@Paulb23
Copy link
Member Author

Paulb23 commented Aug 2, 2021

Yeah I saw that too. It's not caused by this PR as it's the same in master.

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.

Well, everything else seems fine (at least in the editor). Code is ok too.

@akien-mga akien-mga merged commit c620ede into godotengine:master Aug 2, 2021
@akien-mga
Copy link
Member

Thanks!

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.

Minor usability issue with triple quotes
3 participants