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

Use simplified keymap for dumb terminal #874

Merged
merged 2 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions reader/src/main/java/org/jline/reader/LineReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ public interface LineReader {
String MAIN = "main";
String EMACS = "emacs";
String SAFE = ".safe";
String DUMB = "dumb";
String MENU = "menu";

//
Expand Down
15 changes: 13 additions & 2 deletions reader/src/main/java/org/jline/reader/impl/LineReaderImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -5997,6 +5997,8 @@ public Map<String, KeyMap<Binding>> defaultKeyMaps() {
keyMaps.put(VIOPP, viOpp());
keyMaps.put(VISUAL, visual());
keyMaps.put(SAFE, safe());
keyMaps.put(DUMB, dumb());

if (getBoolean(BIND_TTY_SPECIAL_CHARS, true)) {
Attributes attr = terminal.getAttributes();
bindConsoleChars(keyMaps.get(EMACS), attr);
Expand All @@ -6007,8 +6009,9 @@ public Map<String, KeyMap<Binding>> defaultKeyMaps() {
keyMap.setUnicode(new Reference(SELF_INSERT));
keyMap.setAmbiguousTimeout(getLong(AMBIGUOUS_BINDING, DEFAULT_AMBIGUOUS_BINDING));
}
// By default, link main to emacs
keyMaps.put(MAIN, keyMaps.get(EMACS));
// By default, link main to emacs unless the temrinal is dumb
keyMaps.put(MAIN, keyMaps.get(isTerminalDumb() ? DUMB : EMACS));

return keyMaps;
}

Expand Down Expand Up @@ -6263,6 +6266,14 @@ public KeyMap<Binding> safe() {
return safe;
}

public KeyMap<Binding> dumb() {
KeyMap<Binding> dumb = new KeyMap<>();
bind(dumb, SELF_INSERT, range("^@-^?"));
bind(dumb, ACCEPT_LINE, "\r", "\n");
Copy link
Member

Choose a reason for hiding this comment

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

Would adding "\r\n" as an additional key binding help with the cr/lf issue on dumb terminals ?

Copy link
Contributor Author

@hvesalai hvesalai Oct 19, 2023

Choose a reason for hiding this comment

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

That would not be deterministic then. How will the system know if to expect only \r or \r\n. Remember that there might be no immediate input after the \r. Consider the following interaction:

Time n: user inputs something that triggers SELF_INSERTs, such as "Hello"
Time n+1: user inputs \r.
Time n+m: user inputs a new thing that trigger SELF_INSERTs, not being \n, for example, "World"

Surely we cannot wait for a period of unkown m to determine whether the \r at n+1 should have triggered ACCEPT_LINE.

Copy link
Contributor Author

@hvesalai hvesalai Oct 19, 2023

Choose a reason for hiding this comment

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

Consider above with this user over a slow connection:

Time n: process is piped something that triggers SELF_INSERTs, such as "Hello"
Time n+1: process is piped \r
Time n+m: process is piped \n

Waiting for the \n here would be possible (using a short timeout > m), but that would mean that a dumb terminal would always wait after a \r to see if there is a \n following it. Sometimes it could be available, sometimes not. The bottom line is that it would in any case slow down the process.

bind(dumb, BEEP, ctrl('G'));
return dumb;
}

public KeyMap<Binding> visual() {
KeyMap<Binding> visual = new KeyMap<>();
bind(visual, UP_LINE, key(Capability.key_up), "k");
Expand Down
Loading