Skip to content

Commit

Permalink
vi: Fix recalling lines starting with whitespace (re: 161139d)
Browse files Browse the repository at this point in the history
@jghub reports:
> if for some reason a command is entered with leading whitespace,
> the leading whitespace is preserved in the history file (in
> ksh93u+, too). such commands can be recalled from history in 93u+
> without problem, both, in vi and emacs mode.
>
> but in 93u+m such entries are simply ignored when using vi-mode
> (set -o vi), i.e. the k and j commands skip over such entries.
> emacs mode still works just fine.

Analysis: the blankline() function in vi.c and emacs.c is used to
check for whitespace up to the cursor position. This is used for
tab completion and fullscreen editor invocation, where that is
okay, and for history recall, where that should be checking the
entire line instead.

In emacs.c this was not really a problem because arrowing up (or
^P) positions the cursor to the end of the line anyway. But in vi,
arrowing up (or 'k') positions the cursor at the beginning.

src/cmd/ksh93/edit/vi.c:
- Amend blankline() with an 'uptocursor' argument. If nonzero, it
  checks the current line for whitespace up to the cursor position,
  otherwise it checks the entire line.
- Pass uptocursor==0 to blankline() for history recall and 1 for
  tab completion.

src/cmd/ksh93/edit/emacs.c:
- Apply the same change here for consistency, even though it's
  perhaps not really necessary. If ever a form of recall is added
  that doesn't position the cursor at the end, then that won't
  introduce this bug there.

Thanks to @pghvlaans for input into the fix.
Resolves: #799
  • Loading branch information
McDutchie committed Dec 5, 2024
1 parent de55362 commit 2e35789
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 16 deletions.
6 changes: 6 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ This documents significant changes in the 1.0 branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2024-12-05:

- Fixed an issue in the vi line editors where lines beginning with whitespace
could not be recalled from the history (e.g. with arrow up). Bug introduced
on 2022-10-31.

2024-11-26:

- The -v/--verbose option of the cp, mv and ln built-in commands (which are
Expand Down
14 changes: 7 additions & 7 deletions src/cmd/ksh93/edit/emacs.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ static void search(Emacs_t*,genchar*,int);
static void setcursor(Emacs_t*,int, int);
static void show_info(Emacs_t*,const char*);
static void xcommands(Emacs_t*,int);
static char blankline(Emacs_t*, genchar*);
static int blankline(Emacs_t*, genchar*, int);

int ed_emacsread(void *context, int fd,char *buff,int scend, int reedit)
{
Expand Down Expand Up @@ -716,9 +716,9 @@ int ed_emacsread(void *context, int fd,char *buff,int scend, int reedit)
cur = eol;
draw(ep,UPDATE);
/* skip blank lines when going up/down in history */
if(c==cntl('N') && hline != histlines && blankline(ep,out))
if(c==cntl('N') && hline != histlines && blankline(ep,out,0))
ed_ungetchar(ep->ed,cntl('N'));
else if(c==cntl('P') && hline != hismin && blankline(ep,out))
else if(c==cntl('P') && hline != hismin && blankline(ep,out,0))
ed_ungetchar(ep->ed,cntl('P'));
continue;
}
Expand Down Expand Up @@ -1007,7 +1007,7 @@ static int escape(Emacs_t* ep,genchar *out,int count)
case '*': /* filename expansion */
case '=': /* escape = - list all matching file names */
{
if(cur<1 || blankline(ep,out))
if(cur<1 || blankline(ep,out,1))
{
beep();
return -1;
Expand Down Expand Up @@ -1296,7 +1296,7 @@ static void xcommands(Emacs_t *ep,int count)
case cntl('E'): /* invoke emacs on current command */
if(eol>=0 && sh.hist_ptr)
{
if(blankline(ep,drawbuff))
if(blankline(ep,drawbuff,1))
{
cur = 0;
eol = 1;
Expand Down Expand Up @@ -1769,11 +1769,11 @@ static int _isword(int c)
/*
* determine if the command line is blank (empty or all whitespace)
*/
static char blankline(Emacs_t *ep, genchar *out)
static int blankline(Emacs_t *ep, genchar *out, int uptocursor)
{
int x;
ep->mark = cur;
for(x=0; x < cur; x++)
for(x=0; uptocursor ? (x < cur) : (x <= eol); x++)
{
#if SHOPT_MULTIBYTE
if(!iswspace((wchar_t)out[x]))
Expand Down
16 changes: 8 additions & 8 deletions src/cmd/ksh93/edit/vi.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ typedef struct _vi_

static const char paren_chars[] = "([{)]}"; /* for % command */

static char blankline(Vi_t*);
static int blankline(Vi_t*, int);
static void cursor(Vi_t*, int);
static void del_line(Vi_t*,int);
static int getcount(Vi_t*,int);
Expand Down Expand Up @@ -668,9 +668,9 @@ static int cntlmode(Vi_t *vp)
}
#endif /* SHOPT_EDPREDICT */
/* skip blank lines when going up/down in history */
if((c=='k' || c=='-') && curhline != histmin && blankline(vp))
if((c=='k' || c=='-') && curhline != histmin && blankline(vp,0))
ed_ungetchar(vp->ed,'k');
else if((c=='j' || c=='+') && curhline != histmax && blankline(vp))
else if((c=='j' || c=='+') && curhline != histmax && blankline(vp,0))
ed_ungetchar(vp->ed,'j');
break;

Expand All @@ -694,7 +694,7 @@ static int cntlmode(Vi_t *vp)
case 'v':
if(vp->repeat_set==0)
{
if(blankline(vp) || cur_virt == INVALID)
if(blankline(vp,1) || cur_virt == INVALID)
{
cur_virt = 0;
last_virt = cur_virt;
Expand Down Expand Up @@ -1297,7 +1297,7 @@ static void getline(Vi_t* vp,int mode)
{
if(!sh_isoption(SH_VI) || !sh.nextprompt)
goto fallback;
if(blankline(vp))
if(blankline(vp,1))
{
ed_ringbell();
break;
Expand Down Expand Up @@ -2289,7 +2289,7 @@ static int textmod(Vi_t *vp,int c, int mode)
case '\\': /** do file name completion in place **/
case '=': /** list file name expansions **/
{
if(cur_virt == INVALID || blankline(vp))
if(cur_virt == INVALID || blankline(vp,1))
return BAD;
/* FALLTHROUGH */
save_v(vp);
Expand Down Expand Up @@ -2645,10 +2645,10 @@ static int textmod(Vi_t *vp,int c, int mode)
/*
* determine if the command line is blank (empty or all whitespace)
*/
static char blankline(Vi_t *vp)
static int blankline(Vi_t *vp, int uptocursor)
{
int x;
for(x=0; x <= cur_virt; x++)
for(x=0; x <= (uptocursor ? cur_virt : last_virt); x++)
{
#if SHOPT_MULTIBYTE
if(!iswspace((wchar_t)virtual[x]))
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include <ast_release.h>
#include "git.h"

#define SH_RELEASE_DATE "2024-12-03" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-12-05" /* must be in this format for $((.sh.version)) */
/*
* This comment keeps SH_RELEASE_DATE a few lines away from SH_RELEASE_SVER to avoid
* merge conflicts when cherry-picking dev branch commits onto a release branch.
Expand Down
12 changes: 12 additions & 0 deletions src/cmd/ksh93/tests/pty.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1337,5 +1337,17 @@ w print Exit status $?
u ^Exit status 0\r\n$
!

((SHOPT_VSH)) && tst $LINENO <<"!"
L 'k' skips over history entries starting with whitespace
# https://github.com/ksh93/ksh/issues/799
d 40
p :test-1:
w true
p :test-2:
c \Ek
r :test-2: true
!

# ======
exit $((Errors<125?Errors:125))

0 comments on commit 2e35789

Please sign in to comment.