From: Stuart Henderson <stu@spacehopper.org>
Subject: Re: Fix for vi editing mode in sftp(1) (PING)
To: Walter Alejandro Iglesias <wai@roquesor.com>, Damien Miller <djm@mindrot.org>, tech@openbsd.org
Date: Tue, 20 Jan 2026 12:33:53 +0000

On 2026/01/20 12:57, Crystal Kolipe wrote:
> On Tue, Jan 20, 2026 at 12:42:42PM +0100, Walter Alejandro Iglesias wrote:
> > --- sftp.c	13 Oct 2025 00:54:29 -0000	1.247
> > +++ sftp.c	20 Jan 2026 11:06:05 -0000
> > @@ -2204,6 +2204,7 @@ interactive_loop(struct sftp_conn *conn,
> >  	char *remote_path;
> >  	char *dir = NULL, *startdir = NULL;
> >  	char cmd[2048];
> > +	const char *editor = "emacs";
> >  	int err, interactive;
> >  	EditLine *el = NULL;
> >  	History *hl = NULL;
> > @@ -2220,7 +2221,7 @@ interactive_loop(struct sftp_conn *conn,
> >  		el_set(el, EL_HIST, history, hl);
> >  
> >  		el_set(el, EL_PROMPT, prompt);
> > -		el_set(el, EL_EDITOR, "emacs");
> > +		el_set(el, EL_EDITOR, editor);
> >  		el_set(el, EL_TERMINAL, NULL);
> >  		el_set(el, EL_SIGNAL, 1);
> >  		el_source(el, NULL);
> 
> The initial assignment of "emacs" to editor and then using it instead of the
> fixed string in el_set() does nothing useful from a technical point of view.
> 
> Unless you think it improves code readability, by moving the string to the top
> of the function declaration, then Stuart's version of the patch is more
> correct.
> 
> (And in general, for longer and more complex code where it might be useful,
>  improving readability in that way is usually better done with a define rather
>  than a variable assignment.)

I think using the same variable for two different purposes makes
things more confusing.

I do have a small improvement to the previous diff (changed the
comment, no code change).

Index: sftp.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sftp.c,v
diff -u -p -r1.247 sftp.c
--- sftp.c	13 Oct 2025 00:54:29 -0000	1.247
+++ sftp.c	20 Jan 2026 12:33:25 -0000
@@ -2204,6 +2204,7 @@ interactive_loop(struct sftp_conn *conn,
 	char *remote_path;
 	char *dir = NULL, *startdir = NULL;
 	char cmd[2048];
+	const char *editor;
 	int err, interactive;
 	EditLine *el = NULL;
 	History *hl = NULL;
@@ -2239,6 +2240,10 @@ interactive_loop(struct sftp_conn *conn,
 		el_set(el, EL_BIND, "\\e\\e[D", "ed-prev-word", NULL);
 		/* make ^w match ksh behaviour */
 		el_set(el, EL_BIND, "^w", "ed-delete-prev-word", NULL);
+
+		/* el_source() may have changed EL_EDITOR to vi */
+		if (el_get(el, EL_EDITOR, &editor) == 0 && editor[0] == 'v')
+			el_set(el, EL_BIND, "^[", "vi-command-mode", NULL);
 	}

 	if ((remote_path = sftp_realpath(conn, ".")) == NULL)