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 && and & syntax highlighting by altering StringName highlighting #100575

Conversation

Wierdox
Copy link
Contributor

@Wierdox Wierdox commented Dec 18, 2024

Closes godotengine/godot-proposals#11381

Not much to say about this PR, new code should be fairly easy to understand. Check out the proposal for an explanation and some details. That said, it could probably be cleaned up a bit, I always seem to miss something.

Image

@Wierdox Wierdox requested a review from a team as a code owner December 18, 2024 18:27
@Wierdox Wierdox changed the title Improve '&&' and '&' syntax highlighting by altering StringName highlighting Improve && and & syntax highlighting by altering StringName highlighting Dec 18, 2024
@Mickeon Mickeon added this to the 4.x milestone Dec 19, 2024
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

I approve in principle as this looks like an obvious improvement to what can be seen as an oversight. But someone savvy in GDScript should take a look at it, perhaps stress-test it.

@Mickeon Mickeon requested review from HolonProduction and removed request for HolonProduction December 21, 2024 10:48
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

The improvement is great

@dalexeev @HolonProduction do we have any testing facilities for the highlighter? If so there should probably be test cases for this, or if we are working on that functionality this should be accounted for if merged

modules/gdscript/editor/gdscript_highlighter.cpp Outdated Show resolved Hide resolved
@Mickeon Mickeon modified the milestones: 4.x, 4.4 Dec 21, 2024
@dalexeev
Copy link
Member

do we have any testing facilities for the highlighter?

@Wierdox Wierdox force-pushed the improve_syntax_highlighting_for_shorthand_of_and_plus_bitwise_and_by_altering_string_name_highlighting branch from 667dc91 to ed81a17 Compare December 21, 2024 22:02
Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

In the absence of a clear testing method, I'm fine approving this at face-value & covering potential regressions manually

@Repiteo Repiteo merged commit 0f95e9f into godotengine:master Dec 23, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 23, 2024

Thanks!

@Wierdox Wierdox deleted the improve_syntax_highlighting_for_shorthand_of_and_plus_bitwise_and_by_altering_string_name_highlighting branch December 24, 2024 09:11
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.

Improve && and & syntax highlighting by altering StringName highlighting
5 participants