Skip to content

Commit

Permalink
New centralised version of local line editing.
Browse files Browse the repository at this point in the history
This takes over from both the implementation in ldisc.c and the one in
term_get_userpass_input, which were imperfectly duplicating each
other's functionality. The new version should be more consistent
between the two already, and also, it means further improvements can
now be made in just one place.

In the course of this, I've restructured the inside of ldisc.c by
moving the input_queue bufchain to the other side of the translation
code in ldisc_send. Previously, ldisc_send received a string, an
optional 'dedicated key' indication (bodgily signalled by a negative
length) and an 'interactive' flag, translated that somehow into a
combination of raw backend output and specials, and saved the latter
in input_queue. Now it saves the original (string, dedicated flag,
interactive flag) data in input_queue, and doesn't do the translation
until the data is pulled back _out_ of the queue. That's because the
new line editing system expects to receive something much closer to
the original data format.

The term_get_userpass_input system is also substantially restructured.
Instead of ldisc.c handing each individual keystroke to terminal.c so
that it can do line editing on it, terminal.c now just gives the Ldisc
a pointer to its instance of the new TermLineEditor object - and then
ldisc.c can put keystrokes straight into that, in the same way it
would put them into its own TermLineEditor, without having to go via
terminal.c at all. So the term_get_userpass_input edifice is only
called back when the line editor actually delivers the answer to a
username or password prompt.

(I considered not _even_ having a separate TermLineEditor for password
prompts, and just letting ldisc.c use its own. But the problem is that
some of the behaviour differences between the two line editors are
deliberate, for example the use of ^D to signal 'abort this prompt',
and the use of Escape as an alternative line-clearing command. So
TermLineEditor has a flags word that allows ldisc and terminal to set
it up differently. Also this lets me give the two TermLineEditors a
different vtable of callback functions, which is a convenient way for
terminal.c to get notified when a prompt has been answered.)

The new line editor still passes all the tests I wrote for the old
one. But it already has a couple of important improvements, both in
the area of UTF-8 handling:

Firstly, when we display a UTF-8 character on the terminal, we check
with the terminal how many character cells it occupied, and then if
the user deletes it again from the editing buffer, we can emit the
right number of backspace-space-backspace sequences. (The old ldisc
line editor incorrectly assumed all Unicode characters had terminal
with 1, partly because its buffer was byte- rather than character-
oriented and so it was more than enough work just finding where the
character _start_ was.)

Secondly, terminal.c's userpass line editor would never emit a byte in
the 80-BF range to the terminal at all, which meant that nontrivial
UTF-8 characters always came out as U+FFFD blobs!
  • Loading branch information
sgtatham committed Mar 4, 2023
1 parent 7a48837 commit 1a7e4ec
Show file tree
Hide file tree
Showing 10 changed files with 989 additions and 553 deletions.
5 changes: 3 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ add_library(agent STATIC

add_library(guiterminal STATIC
terminal/terminal.c terminal/bidi.c
ldisc.c config.c dialog.c
ldisc.c terminal/lineedit.c config.c dialog.c
$<TARGET_OBJECTS:logging>)

add_library(noterminal STATIC
Expand Down Expand Up @@ -115,7 +115,8 @@ add_executable(bidi_test
target_link_libraries(bidi_test guiterminal utils ${platform_libraries})

add_executable(plink
${platform}/plink.c)
${platform}/plink.c
stubs/no-lineedit.c)
# Note: if we ever port Plink to a platform where we can't implement a
# serial backend, this be_list command will need to become platform-
# dependent, so that it only sets the SERIAL option on platforms where
Expand Down
5 changes: 5 additions & 0 deletions defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ typedef struct LogContext LogContext;
typedef struct LogPolicy LogPolicy;
typedef struct LogPolicyVtable LogPolicyVtable;

typedef struct TermLineEditor TermLineEditor;
typedef struct TermLineEditorCallbackReceiver TermLineEditorCallbackReceiver;
typedef struct TermLineEditorCallbackReceiverVtable
TermLineEditorCallbackReceiverVtable;

typedef struct Seat Seat;
typedef struct SeatVtable SeatVtable;
typedef struct SeatDialogText SeatDialogText;
Expand Down
Loading

0 comments on commit 1a7e4ec

Please sign in to comment.