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

Abstracted the syntax highlighter from text edit. #17923

Merged

Conversation

Paulb23
Copy link
Member

@Paulb23 Paulb23 commented Apr 2, 2018

This PR abstracts out the syntax highlighting from the draw section of TextEdit allowing custom highlighters to be used.

There is a new class called SyntaxHighlighter of which all syntax highlighters must extend. All TextEdits and SyntaxHighlighters have a 1 to 1 relationship, so a SyntaxHighlighter cannot serve multiple TextEdits and vice versa.

If the TextEdit is not given a SyntaxHighlighter or is set to NULL it will go ahead a use a modified version of the current system.

The SyntaxHighlighter::_get_line_syntax_highlighting will get called for every line that we are drawing. Returning Map<int, HighlighterInfo> where the key is the column start of the section and HighlighterInfo a new struct directing how this section should be drawn. Currently it can only change the colour. The section ends when the end of line is reached or a new section is found.

There is also a new function in ScriptEditorPlugin to register SyntaxHighlighter creation functions. Now when a new ScriptEditor is created the SyntaxHighlighters will also be created and added.

When adding SyntaxHighlighters it will check if it supports the scripts language, if a match is found that highlighter will be automatically selected.

The ScriptTextEditor has gained a new drop down menu item Syntax Highlighter listing all the added SyntaxHighlighters giving you the ability to change the current highlighter on the fly.

I've also included a GDSyntaxHighlighter that is currently identical to the default one, ready for adding GDScript specific use cases.

Lastly, in the process I've had to remove the encapsulation around some of the internal TextEdit implementation and hopefully made calculating ColorRegion(s) slightly more performance friendly.

closes #15691

@akien-mga akien-mga added this to the 3.1 milestone Apr 3, 2018

#include "scene/gui/text_edit.h"

class GDSyntaxHighlighter : public SyntaxHighlighter {
Copy link
Member

Choose a reason for hiding this comment

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

It should be GDScriptSyntaxHighlighter, we've renamed all GD* classes to GDScript* to prevent the confusion with GDNative (e.g. the previous GDNativeClass was actually a GDScript class).

@reduz
Copy link
Member

reduz commented Apr 3, 2018

How about multine highlightnig? for example:

'''
/* Some comment
like this
*/
'''
the shader editor currently uses it


int in_region = -1;
int deregion = 0;
for (int i = 0; i < p_line; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

It seems that going through the whole text for getting a line color is pretty inefficient. There's probably a better way to handle multiple line coloring.

Copy link
Member

Choose a reason for hiding this comment

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

I think doing this is fine, but a caching system with proper invalidation should be used instead. Ie, if you edit line 5, the highlighting for all lines below 5 becomes invalid.

Copy link
Member

Choose a reason for hiding this comment

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

A line cache should simply remember the hlighliter computed status you already have in that line, and the last valid line, so you just run from that line onwards when redrawing. Should be very efficient even for very large files.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess great minds think alike ;)

I have some changes locally around the idea of caching then dropping everything below and including the edited line. Was going to send that in a separate PR when I can get some more time to finish it.

However, yeah at the moment it's not the most efficient solution, but should be good enough.

@akien-mga akien-mga merged commit 5ede505 into godotengine:master Apr 4, 2018
@Paulb23 Paulb23 deleted the add_abstract_syntax_highlighter branch April 4, 2018 20:47
@Faless
Copy link
Collaborator

Faless commented Apr 5, 2018

Something seems wrong with this PR

(Screenshot of the multiplayer bomber demo, everything after the what's going on comment is treated as a string, probably not ignoring quotes in comments properly)
highlighter

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.

TextEdit has add_color_region, but no way to get colors
6 participants