GDScript: Highlight comment markers (TODO, FIXME, etc.)

[dalexeev]

• Closes Highlight TODO, FIXME and similar keywords in comments in the script editor godot-proposals#4942.

[MewPurPur]

Does it look good on light theme, where comments appear brighter rather than darker?

[dalexeev]

The light theme looks like this:

I didn't choose these colors carefully, in the hope that someone with better taste would suggest other colors.

[dalexeev]

Dark

Godot 2

Light

[MewPurPur]

Tbh I like them, but I'd like it if a similar result for dark theme could get achieved with something like 0.6 alpha so it's more comment-like in all themes.

[Calinou]

Tested locally (rebased on top of master 6588a4a), it works as expected.

See this example with the text from the original proposal for reference:

I like the use of multiple colors, it makes sense here.

The implementation itself looks good to me from a cursory look. I've also tested scenarios such as comments inside strings, and can confirm no highlighting is applied there (it shouldn't).

Changes to the list of keywords in the editor settings also work correctly without requiring a restart.

[dalexeev]

I'd like it if a similar result for dark theme could get achieved with something like 0.6 alpha so it's more comment-like in all themes

Indeed, the text_editor/theme/highlighting/comment_color editor setting has a translucent color.

But I checked the default themes and the colors look good in my opinion. I'm not sure if non-grayscale translucent colors will work for any theme. Let me know if you think it's worth looking into.

Notes:

1. In this implementation, markers must consist of ASCII identifier characters. Should we add support for Unicode markers? To do this, I just need to replace is_ascii_identifier_char() with is_unicode_identifier_continue(), but I have performance concerns. On the other hand, this only applies to comments and this function will be optimized in the future, see Add Unicode support to String.to_*_case() methods #75846.
2. I see an opportunity to use more than 3 hardcoded levels here. We can use one editor setting of type Dictionary "list of markers -> color". This shouldn't affect performance since we have a HashMap for marker colors. Is it worth it, or "three levels is enough for everyone"?
3. This PR only implements it for GDScript. In the future, we should add this for C# and the shader language too.

[Calinou]

2. I see an opportunity to use more than 3 hardcoded levels here. We can use one editor setting of type Dictionary "list of markers -> color". This shouldn't affect performance since we have a HashMap for marker colors. Is it worth it, or "three levels is enough for everyone"?

I would wait until there's significant demand before implementing more settings (likely in a separate proposal if this PR is merged).

[dalexeev]

Here is how it might look (dalexeev@46fb4c1):

The disadvantage is that you cannot reset the colors individually.

[Bromeon]

Some input on splitting into 3 groups, but that's bikeshedding territory 😬

• HACK seems more like FIXME, TODO i.e. something that should be cleaned up, but is not critical
• maybe CRITICAL could be added to the critical list
• maybe INFO could be added to the notice list
• not fully sure about difference between CAUTION and ATTENTION? (probably a native speaker knows better)

[dalexeev]

Some input on splitting into 3 groups

Done.

1. Should we add support for Unicode markers?

Done.

I also tweaked the colors a bit so they are brighter/clearer but still dimmer than the keywords. The light theme colors don't seem very good to me, if there are any suggestions I'll fix it.

[Calinou]

I also tweaked the colors a bit so they are brighter/clearer but still dimmer than the keywords. The light theme colors don't seem very good to me, if there are any suggestions I'll fix it.

The new colors look great to me, even on light theme 🙂

[akien-mga]

Seems fine to me. Didn't review the highlighter code in depth but it's relatively low risk.
The feature proposal seems well appreciated by the community.

[akien-mga]

Thanks!