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

ImGuiInputTextFlags_AlwaysInsertMode forces overwrite mode instead of insert mode #2863

Open
jstine35 opened this issue Oct 22, 2019 · 4 comments

Comments

@jstine35
Copy link

When applying ImGuiInputTextFlags_AlwaysInsertMode to an InputText, the observed behavior is overwrite rather insert.

I believe the usefulness of the flag currently is minimal, since there's no built-in support for toggling insert/overwrite modes. This would explain it going unreported, since the workaround is literally "don't use that flag." Even so, it would be helpful if the flag's behavior were corrected so that it, at a minimum, doesn't enforce overwrite behavior.

Relevant code snippet in imgui_widgets.cpp:

        if (flags & ImGuiInputTextFlags_AlwaysInsertMode)
            state->Stb.insert_mode = 1;

... the culprit being that STB's insert_mode flag is 0 for insert, and 1 for overwrite. Changing the above line to 0 resolves the most confusing aspect of this issue.

@ocornut
Copy link
Owner

ocornut commented Oct 22, 2019

Thank you @jstine35 for pointing this out.
Indeed the bug/problem comes from the incorrect name being used by stb_textedit which I carried over, assuming "insert mode" meant "overwrite". A cursory Google check confirms that the name is wrong.

It should be called ImGuiInputTextFlags_AlwaysOverwriteMode.
The feature as is, minus the wrong name, is useful and used e.g. by imgui_memory_editor. Unfortunately because of this I am forced to maintain the old (incorrect) name for a little while.

So the idea is that we should fix the name rather than changing the behavior. It is expected that the default behavior of an InputText is to "insert" text.

Action:

  • rename to ImGuiInputTextFlags_AlwaysOverwriteMode. keep old name with a bunch of comments in uppercase and under a #ifndef IMGUI_DISABLE_OBSOLETE_FUNCTIONS block.

I think the reason I didn't add support for Insert key actually toggling this is because I wasn't happy with the current behavior of stb_textedit.h when in overwrite mode and reaching the end of a line.

The line that does this check:
if (state->insert_mode && !STB_TEXT_HAS_SELECTION(state) && state->cursor < STB_TEXTEDIT_STRINGLEN(str)) {
Should allow check that we are not at the end of a line IMHO.

I will try to look into it eventually, would would happily accept a patch. I think the patch should probably be also pushed to the nothings/stb repo, including renaming the insert_mode field to overwrite_mode. Because stb_textedit.h has few users I imagine it may be possible to suggest breaking API on this one.

@jstine35
Copy link
Author

Thanks for the clarification with MemoryView. Indeed, I had bugged up our MemoryView by changing ImGui internal behavior. I've gone and removed the errant uses of ImGuiInputTextFlags_AlwaysInsertMode from the rest of our codebase instead. :)

I had thought that stb_textedit logic seemed suspicious too, though I think I was still being misled by the insert_mode meaning mismatching its name. The condition falls through to insert mode if any of those checks fails. It doesn't actually reject any keystrokes so, I think, the logic is OK as it is?

  • If the cursor is at the EOL and that check fails, then it will insert (append) new chars to the end of the line.
  • If the char buffer is full then insert also fails and no keystroke is recorded.

So. I believe that should be the expected behavior?

@ocornut ocornut added the bug label Mar 12, 2021
ocornut added a commit that referenced this issue Mar 12, 2021
ocornut added a commit that referenced this issue Mar 12, 2021
@ocornut
Copy link
Owner

ocornut commented Mar 12, 2021

FYI I pushed renaming this flag from ImGuiInputTextFlags_AlwaysInsertMode to ImGuiInputTextFlags_AlwaysOverwrite.

As for actual handling of Insert key in text field I'll work on this separately (also added to todo list).

  • We would need dedicated cursor rendering which given the shape may need a color :( may hardcode to lower alpha of text. and have the cursor change when on a new-line character.
  • We would need to maintain a shared state for that toggle (trivial).

I had thought that stb_textedit logic seemed suspicious too, though I think I was still being misled by the insert_mode meaning mismatching its name. The condition falls through to insert mode if any of those checks fails. It doesn't actually reject any keystrokes so, I think, the logic is OK as it is?
If the cursor is at the EOL and that check fails, then it will insert (append) new chars to the end of the line.
If the char buffer is full then insert also fails and no keystroke is recorded.

I'm not sure I understood what you meant there. Failure due to full buffer shouldn't happen when overwriting?

@ocornut
Copy link
Owner

ocornut commented Mar 12, 2021

This branch 30f2ca8 adds support for INSERT key to toggle overwrite mode.
It seems to work well in single and multi-line mode.

My problem right now is I am not 100% sure if:

  • should be opt-in (e.g. ImGuiInputText_AllowOverwriteMode)
  • should be opt-out (e.g. ImGuiInputText_NoOverwriteMode)

In the context of a text editor it seems like an always available thing to have this toggle on the Insert key.
But if I look on Windows or right now in the GitHub text edit box, or FireFox address box, it is not available. Why so? Is Insert frequently used for other features?

Also, making it opt-in would facilitate people creating application where things feels inconsistent to end user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants