Tickets #4872, #4873 and #4874: fix 'insert control code' function by peripherium · Pull Request #5054 · MidnightCommander/mc

peripherium · 2026-03-08T09:25:56Z

The previous implementation used ascii_alpha_to_cntrl(), which assumed
alphabetic input and did not properly validate key codes returned by the
terminal libraries. The function has been renamed to keycode_to_cntrl()
and extended to correctly validate and convert key codes.

Changes

correctly handle KEY_M_CTRL combinations (e.g. Ctrl+J).
Fixes mcedit: Display corruption (missing header) after inserting literal ^J #4872.
ignore special keys returned by get_key_code() (e.g. function or cursor keys).
Fixes mcedit: Insert literal, followed by special key acts weirdly #4873.
extend control mappings for @ [ \ ] ^ _.
Fixes mcedit: Insert literal should accept plain non-letters #4874.
return -1 for invalid input and ignore it at the call sites

The function is used both in the Insert literal dialog in mcedit
and in the mc command line.

Checklist

I have referenced the issue(s) resolved by this PR (if any)
I have signed-off my contribution with git commit --amend -s
Lint and unit tests pass locally with my changes (make indent && make check)
I have added tests that prove my fix is effective or that my feature works
I have added the necessary documentation (if appropriate)

zyv · 2026-03-08T14:21:26Z

Since you are fixing keycode_to_cntrl, could you please add a test for it? Thank you!

zyv · 2026-03-08T14:23:09Z

@egmontkob adding you, since this PR fixes 3 issues that you have reported...

peripherium · 2026-03-09T10:40:54Z

I initially didn't add a test for this function because the previous implementation didn't have one, so I assumed that adding a test was not required.

In the meantime, I have taken a closer look at the test suite and added the requested test here.

If this test is not sufficient or needs adjustments, please let me know.

mc-worker

Formatting issues:
https://github.com/MidnightCommander/mc/actions/runs/22849421605/job/66290234418

lib/widget/input.c

tests/lib/Makefile.am

tests/lib/util__keycode_to_cntrl.c

egmontkob

The commit message is duplicated.

egmontkob · 2026-03-10T09:15:24Z

lib/util.c

-    if ((ch >= ASCII_A && ch <= ASCII_Z) || (ch >= ASCII_a && ch <= ASCII_z))
-        ch &= 0x1f;
+    // Only KEY_M_CTRL and lowest 7 bits of ch may be set
+    if ((ch & KEY_M_CTRL) != 0 && (ch & ~(KEY_M_CTRL | 0x7f)) != 0)


The condition doesn't match the comment above.

egmontkob · 2026-03-10T09:18:29Z

lib/util.c

+        return ch - '`';

-    return ch;
+    return -1;


Shouldn't you also handle the common practice of Ctrl+? as 0x7F ?

egmontkob · 2026-03-10T09:20:10Z

lib/util.h

@@ -264,7 +264,7 @@ void save_file_position (const vfs_path_t *filename_vpath, long line, long colum

 /* if ch is in [A-Za-z], returns the corresponding control character,


You should update this comment with the additional characters you handle now; also the modified "else" return value.

egmontkob · 2026-03-10T09:21:45Z

lib/util.c

+    // Only KEY_M_CTRL and lowest 7 bits of ch may be set
+    if ((ch & KEY_M_CTRL) != 0 && (ch & ~(KEY_M_CTRL | 0x7f)) != 0)
+        return -1;
+


Trailing spaces here, please run make indent.

egmontkob · 2026-03-10T09:26:47Z

lib/util.c

+    if (ch >= '@' && ch <= '_')
+        return ch - '@';
+
+    if (ch >= '`' && ch <= '~')


Do we need to extend beyond the lowercase ASCII chars?

Ctrl+backtick, Ctrl+{ etc. are absolutely unconventional ways to denote the control chars, I don't think we should handle them.

egmontkob

Tested now, and it indeed fixes those three bugs, thanks!

peripherium · 2026-03-11T09:05:09Z

I included the lowercase characters because I thought it would be user-friendly: even if shift wasn't pressed during input, the corresponding control code can be entered via the dialog. However, this is a bit inconsistent with the last inserted condition, according to which ch == '?' returns 0x7f. Should I remove the conditions with the lowercase characters from the code?

egmontkob · 2026-03-11T09:25:14Z

Lowercase letters are absolutely fine, in fact, expected; the user probably won't bother pressing Shift.

What's unnecessary IMHO is support the characters "surrounding" the lowercase letters in the ASCII table: ` { | } ~. Here it's absolutely reasonable that the user will instead press @ [ \ ] ^ (or _ which didn't have a counterpart).

Validate input key codes, ignore special keys returned by get_key_code(). Fix handling of KEY_M_CTRL combinations and extend mapping for '@', '[', '\', ']', '^', '_'. Fixes: MidnightCommander#4872 Fixes: MidnightCommander#4873 Fixes: MidnightCommander#4874 Signed-off-by: Manuel Einfalt <einfalt1@proton.me> lib/util.c (keycode_to_cntrl): Rename from ascii_alpha_to_cntrl. Validate input key codes, ignore special keys returned by get_key_code(). Fix handling of KEY_M_CTRL combinations and extend mapping for '@', '[', '\', ']', '^', '_'. Fixes: MidnightCommander#4872 Fixes: MidnightCommander#4873 Fixes: MidnightCommander#4874 Signed-off-by: Manuel Einfalt <einfalt1@proton.me>

peripherium · 2026-03-12T08:29:55Z

Originally I allowed characters from ´ to ~ because I assumed someone looking at the ASCII table might wonder why those printable characters were excluded.

However, if this is not intuitive and could instead confuse future readers, then it doesn't belong in the code. So I removed it and restricted the range to a-z.

github-actions bot added needs triage Needs triage by maintainers prio: medium Has the potential to affect progress labels Mar 8, 2026

github-actions bot added this to the Future Releases milestone Mar 8, 2026

zyv requested a review from egmontkob March 8, 2026 14:22

peripherium force-pushed the fix/control-sequence-from-keyboard branch from 94db99e to fc84480 Compare March 9, 2026 10:35

mc-worker reviewed Mar 9, 2026

View reviewed changes

lib/widget/input.c Show resolved Hide resolved

tests/lib/Makefile.am Outdated Show resolved Hide resolved

tests/lib/util__keycode_to_cntrl.c Show resolved Hide resolved

tests/lib/util__keycode_to_cntrl.c Show resolved Hide resolved

zyv added area: mcedit mcedit, the built-in text editor and removed needs triage Needs triage by maintainers labels Mar 9, 2026

zyv modified the milestones: Future Releases, 4.9.0 Mar 9, 2026

peripherium force-pushed the fix/control-sequence-from-keyboard branch from fc84480 to 74024a5 Compare March 10, 2026 06:31

egmontkob reviewed Mar 10, 2026

View reviewed changes

peripherium force-pushed the fix/control-sequence-from-keyboard branch from 74024a5 to d97eeee Compare March 11, 2026 08:46

zyv changed the title ~~Ticket #4872, #4873 and #4874: Fix 'insert control code' function.~~ Tickets #4872, #4873 and #4874: fix 'insert control code' function Mar 11, 2026

peripherium force-pushed the fix/control-sequence-from-keyboard branch from d97eeee to 0a7f477 Compare March 12, 2026 08:23

		@@ -264,7 +264,7 @@ void save_file_position (const vfs_path_t *filename_vpath, long line, long colum

		/* if ch is in [A-Za-z], returns the corresponding control character,

Uh oh!

Conversation

peripherium commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

zyv commented Mar 8, 2026

Uh oh!

zyv commented Mar 8, 2026

Uh oh!

peripherium commented Mar 9, 2026

Uh oh!

mc-worker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egmontkob left a comment

Choose a reason for hiding this comment

Uh oh!

egmontkob Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

egmontkob Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

egmontkob Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

egmontkob Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

egmontkob Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

egmontkob left a comment

Choose a reason for hiding this comment

Uh oh!

peripherium commented Mar 11, 2026

Uh oh!

egmontkob commented Mar 11, 2026

Uh oh!

peripherium commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

peripherium commented Mar 8, 2026 •

edited

Loading