Editing the buffer in InputTextCallback desyncs the undo/redo stack (Fix #4947)

[JoshuaWebb]

Implement a function to update the undo/redo state for an InputText
after a user callback has modified the buffer.

This function is a bit weird in that it uses the wide string from the
state for the old text and a utf8 buffer for the new text. It does this
because these are the formats that are already available at the time we
need them without requiring additional storage.

[ocornut]

Looking at code now, it seems incorrect at handling non-ASCII character.

• new_buf + new_last_diff that's mixing UTF-8 data new_buf, and Codepoint count new_last_diff
• new_last_diff -= 1 ditto.

I'll rework this, I think the simplest is to do a single conversion.
(EDIT To clarify it won't crash but will often tend to include the full string from where as an undo step and move the cursor to the end.)

(Ultimately we should transition toward (1) making InsertChars()/DeleteChars() update undo-stack (2) discourage (and keep reconcile code) or obsolete (and remove reconcile code) accessing buffer directly.)

[JoshuaWebb]

Whoops, yeah that's definitely a problem, sorry about that.

As for (1) and (2), I had considered implementing updating the undo state in those functions instead, but that necessitates two undos for a single replacement, so if/when we do go down that route it would be a good idea to add a ReplaceChars() function that only adds a single combined undo record.

Batching/reconciling the undo state after the callback (as in this PR) has the benefit of only creating a single undo per callback regardless of how many/which individual characters were changed.

In general, this more closely aligns with my mental model of the system as a user. For example I, as the user, trigger some callback/action and don't like the result so I press the undo key to undo that action; it feels much better to now be in the state I was in before the callback (in an ideal situation the cursor would also be restored to its pre-callback position) than it does to now be in some intermediate state where I have to keep incrementally pressing undo an unknown amount of times to undo individual changes that the program made. Once that happens enough times it begins to feel adversarial "I keep having to clean up the mess made by this stupid auto-complete" etc.

With an available ReplaceChars() function, one could implement something similar to this PR for their callbacks if that matters to them.

[ocornut]

Also found another issue:
while (old_last_diff >= where && new_last_diff >= 0) the later should definitively be && new_last_diff >= where) otherwise there are situation where result is invalid. Was lucky I noticed it earlier as it happened in my testing.

Here's what the modified function looks like:

// Find the shortest single replacement we can make to get the new text from the old text.
// Important: needs to be run before TextW is rewritten with the new characters because calling STB_TEXTEDIT_GETCHAR() at the end.
// FIXME: Ideally we should transition toward (1) making InsertChars()/DeleteChars() update undo-stack (2) discourage (and keep reconcile) or obsolete (and remove reconcile) accessing buffer directly.
static void InputTextReconcileUndoStateAfterUserCallback(ImGuiInputTextState* state, const char* new_buf_a, int new_length_a)
{
    ImGuiContext& g = *GImGui;
    const ImWchar* old_buf = state->TextW.Data;
    const int old_length = state->CurLenW;
    const int new_length = ImTextCountCharsFromUtf8(new_buf_a, new_buf_a + new_length_a);
    g.TempBuffer.reserve_discard((new_length + 1) * sizeof(ImWchar));
    ImWchar* new_buf = (ImWchar*)g.TempBuffer.Data;
    ImTextStrFromUtf8(new_buf, new_length + 1, new_buf_a, new_buf_a + new_length_a);

    const int shorter_length = ImMin(old_length, new_length);
    int first_diff;
    for (first_diff = 0; first_diff < shorter_length; first_diff++)
        if (old_buf[first_diff] != new_buf[first_diff])
            break;
    if (first_diff == old_length && first_diff == new_length)
        return;

    int old_last_diff = old_length - 1;
    int new_last_diff = new_length - 1;
    for (; old_last_diff >= first_diff && new_last_diff >= first_diff; old_last_diff--, new_last_diff--)
        if (old_buf[old_last_diff] != new_buf[new_last_diff])
            break;

    const int insert_len = new_last_diff - first_diff + 1;
    const int delete_len = old_last_diff - first_diff + 1;
    if (insert_len > 0 || delete_len > 0)
        if (STB_TEXTEDIT_CHARTYPE* p = stb_text_createundo(&state->Stb.undostate, first_diff, delete_len, insert_len))
            for (int i = 0; i < delete_len; i++)
                p[i] = ImStb::STB_TEXTEDIT_GETCHAR(state, first_diff + i);
}

I changed g.TempBuffer to make it resizable for this purpose (it wasn't before).

[ocornut]

Merged 530332d + amends a35e876
Thank you!