Syntax highlighting for InputText and ColoredText

[ggtucker]

Hi all,

I want to highlight syntax in an InputText, so I implemented a potential solution for both InputText and ColoredText. I'm curious what people think, and if there are any ideas for different approaches and/or potential improvements here.

This change would resolve issue #902.

Kinda low quality gif, but here's a demo for good measure :).

[ocornut]

Hello Geoffrey, Good to hear from you and I hope you're all doing fine in California.

My gut feeling is that this approach as-is would largely kill performances when the callback is enabled, which may be good enough for casual use but it would hinder larger adoption. If we want this to be a first-class citizen we'll need something more efficient.

One suggestion would be to allow the callback to communicate "when to call me back" character count to the text render loop. So instead of if (callback) we'd do something like:

// Init
int remaining_chars_until_callback = callback ? 1 : 0;

// Loop
if (--remaining_chars_until_callback == 0)
{
   remaining_chars_until_callback = callback(...);
}

So the callback can return 1 if it absolutely needs to be called for every character, but most likely the "token-parsing" done by the callback would allow to fast-forward and only request callback on the next color change. A marker based scheme could look ahead for the closing marker, etc.

EDIT One extra benefit for the approach is that for very large text it becomes easier for a widget/program to precompute an array of offset/color and just feed that to a simple callback.

My hope is that once we find a satisfying design the code for word-wrapping could rely on the same or a very similar design, which would also allow for custom word-wrapping. (#3002)

PS.1: One issue, which may be solved later (no need to be distracted from the design/specs yet) is that the RenderText() function includes two forward-skipping loops at the beginning of the function, aimed to handle clipping of large blurbs of text. This skipping could lead to missing a color change, and would be to be designed to the callback could act in the ideal manner (e.g. calback could decide to handle a fast skipping as well).

PS.2: As for adding parameters to the function signature, I'm not worrying about this presently, my expectation is that we'll introduce more full-featured text functions and everything will fit in a structure, so adding those extra stuff won't be a problem then. Experimenting with text coloring however is one of the many things we have to do before redesigning the text functions, so this topic is good.

[ggtucker]

Hey Omar,

Things are well enough in California! Definitely ready for this pandemic to be over... I hope everything's ok for you in France as well.

I appreciate the quick response! I think the perf optimization is a great suggestion. I will take a stab at implementing it tonight :).

I had a thought that it could be useful to return two integers: chars_for_color and chars_until_callback. That way you could say how many chars should use the returned color, and how many chars there are until the next token. Any chars in-between would then use the default color (in most cases ImGuiCol_Text).

Cheers, Geoff

[ggtucker]

Is there any easy way to test whether the text buffer has changed (1 char added or removed) from the ImGuiInputTextCallback? This is a bit of a side note but still related because we need to update our token(s) when a modification is made.

I know we can hook into ImGuiInputTextFlags_CallbackCharFilter for detecting a char insertion, but I'm not aware of a hook for a char deletion.

[ocornut]

InputText() will return true - and the timing of the callback you are suggesting won’t be more favorable than the return value, so I’d assume you could use that?

[ggtucker]

InputText() will return true - and the timing of the callback you are suggesting won’t be more favorable than the return value, so I’d assume you could use that?

I think the callback's timing might be more favorable? I think we want to re-tokenize after an edit and before we render the colors of the tokens. Otherwise we may see some subtle 1-frame coloration issues.

[ocornut]

You are right, will come up with something.

[ggtucker]

Hey Omar,

I've just committed the perf optimizations we talked about with the remaining_chars_for_color and remaining_chars_until_callback :).

PS.1: One issue, which may be solved later (no need to be distracted from the design/specs yet) is that the RenderText() function includes two forward-skipping loops at the beginning of the function, aimed to handle clipping of large blurbs of text. This skipping could lead to missing a color change, and would be to be designed to the callback could act in the ideal manner (e.g. calback could decide to handle a fast skipping as well).

I am essentially handling forward-skipping in the demo right now, albeit in a simple way. The existing callback signature may be enough to handle these cases? See my comments here:

static void MyTextColorCallback(ImGuiTextColorCallbackData* data)
{
    ImVector<Token>* my_tokens = (ImVector<Token>*)data->UserData;
    const int char_idx = data->Char - data->TextBegin;

    // this is the user's chance to play catch-up and sift through their tokens
    // until they reach the char being rendered, potentially building 'context' along the way

    // here we jump to the first token that ends after (or at) the current char index
    // in the case where offsets/colors are precomputed, this could be a quick binary search

    while (data->TokenIdx < my_tokens->size() && char_idx >= (*my_tokens)[data->TokenIdx].end)
        ++data->TokenIdx;

    // choose color based on the token now that we're caught up...
}

Here's what the full demo function looks like right now:

static void MyTextColorCallback(ImGuiTextColorCallbackData* data)
{
    ImVector<Token>* my_tokens = (ImVector<Token>*)data->UserData;
    const int char_idx = data->Char - data->TextBegin;
    while (data->TokenIdx < my_tokens->size() && char_idx >= (*my_tokens)[data->TokenIdx].end) // jump to the first token that does not end before the current char index (could binary search here)
        ++data->TokenIdx;
    if (data->TokenIdx >= my_tokens->size()) // all tokens end before the current char index
    {
        data->CharsUntilCallback = 0; // stop calling back
        return;
    }
    if (char_idx < (*my_tokens)[data->TokenIdx].begin)
    {
        data->CharsUntilCallback = (*my_tokens)[data->TokenIdx].begin - char_idx; // wait until we reach the first token
        return;
    }
    // If the current token matches the prefix, color it red
    if (Strnicmp(&data->TextBegin[(*my_tokens)[data->TokenIdx].begin], colored_token_prefix, strlen(colored_token_prefix)) == 0)
    {
        data->Color = IM_COL32(255, 0, 0, 255);
        data->CharsForColor = (*my_tokens)[data->TokenIdx].end - char_idx; // color from the current char to the token end
    }
    // If there is another token, callback once we hit the start of it. otherwise, stop calling back.
    data->CharsUntilCallback = (data->TokenIdx + 1 < my_tokens->size()) ? (*my_tokens)[data->TokenIdx + 1].begin - char_idx : 0;
}

[xNxExOx]

@ocornut 🤔 what is preventing this from being merged? it seems all grean.

[jarikomppa]

I implemented something similar, but in a more hackish way. My two cents:

• Having cursor position in the callback data would be valuable. In my example I hilight matching braces based on the cursor position. I recall seeing other people asking for cursor position in the past.
• The additional field for user data that is not touched by ImGui is a bit weird, but I see the point.
• The primary problem with this patch is the parameter explosion. There should be some other way to set/clear the callback than to add more and more parameters to the functions. I believe that the push/pop mechanism exists for this reason.

I implemented the "don't change color for X characters" in my callback. I suppose it saves a bit of time to do it on the caller end.

[ggtucker]

Hey @ocornut! Any feeling for when we might be able to get in support for syntax highlighting?

I've rebased this PR and fixed it up so it's green for merging again (pending any additional feedback or adjustments on your end).

[ggtucker]

I made one additional change 1b964d9, which was to reduce the number of parameters on InputText. Instead of passing two callbacks, I re-use the ImGuiInputTextCallback to support the text coloration via the ImGuiInputTextFlags_CallbackColor flag.

[jparr721]

Is there any intention to merge this change in the near term? I'd like to use it for something I'm working on.

[ocornut]

Not in the near term, sorry, too many fishes to fry :( But we WILL add ways to support this.