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

Fix uint's suffix is not highlighted in text shader editor #85014

Merged
merged 1 commit into from
May 4, 2024

Conversation

jsjtxietian
Copy link
Contributor

Fixes #75161

GDShaderSyntaxHighlighter uses CodeHighlighter which doesn't recognize this suffix syntax, there are other ways to fix this, IMHO add a bool in the base class to distinguish the behaviour is the one introduces least change.

Chaosus

This comment was marked as outdated.

Copy link
Member

@Chaosus Chaosus left a comment

Choose a reason for hiding this comment

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

Added several suggestions, also set_uint_suffix_enabled might be an open method, but this can be fixed in another PR.

scene/resources/syntax_highlighter.cpp Outdated Show resolved Hide resolved
scene/resources/syntax_highlighter.h Outdated Show resolved Hide resolved
scene/resources/syntax_highlighter.h Outdated Show resolved Hide resolved
scene/resources/syntax_highlighter.cpp Outdated Show resolved Hide resolved
editor/plugins/text_shader_editor.cpp Outdated Show resolved Hide resolved
Copy link
Member

@Chaosus Chaosus left a comment

Choose a reason for hiding this comment

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

LGTM

@akien-mga akien-mga merged commit 35faf50 into godotengine:master May 4, 2024
16 checks passed
@jsjtxietian jsjtxietian deleted the fix-uint-not-highlight branch May 4, 2024 09:57
@akien-mga
Copy link
Member

Thanks!

@@ -139,6 +141,8 @@ class CodeHighlighter : public SyntaxHighlighter {

void set_member_variable_color(Color p_color);
Color get_member_variable_color() const;

void set_uint_suffix_enabled(bool p_enabled = true);
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious as to why this setter has a default parameter. We generally don't use default parameters for setter methods by convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just followed @Chaosus 's suggestion for this, in this case looks like explicit is better

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.

Unsigned int suffix not highlighted in shader editor
6 participants