Skip to content

Commit

Permalink
Fix multiple bugs in emacs and vi (#565)
Browse files Browse the repository at this point in the history
This (somewhat expanded) commit addresses three behaviors in vi
mode and two behaviors in emacs mode:

- In vi mode, calling v from a completely empty line rings the bell
  rather than calling the full editor.
- In vi mode, calling v from a spaces-only line results in an
  "invalid range" hist error, and the editor fails to open.
- In emacs mode, calling the full editor saves the current contents
  of the command line to the history file without a new line.
  (issue #563)
- In both editing modes, navigating to a null or empty history line
  displays an empty line onscreen.

A summary of the fixes:

The hist builtin seems to expect either a new line with the cursor
in position 0 or a line with at least one non-space character; in
all cases, the line should conclude with \n and \0:

- In vi.c, set the position to 0, refresh and insert \n if command
  mode v is called from an empty line or a line made up entirely of
  spaces. This enables hist to call the full editor with an
  appropriate range.
- In emacs.c, add \n and \0 to drawbuff before calling ed_fulledit
  (but after checking its failure conditions). This prevents the
  insertion of null history items for both empty and non-empty
  lines. Because lines composed entirely of spaces can cause the
  following prompt to be offset, clear such lines as well.

Other shells, including mksh, oksh, bash and yash, also save empty
history lines. However, actually displaying these blank lines when
navigating using e.g. the arrow keys (as is the case in ksh, mksh
and oksh) is unhelpful:

- In vi.c and emacs.c, check the history line for non-space
  characters. If the line is completely empty, resend the
  appropriate key provided that doing so would not go beyond the
  confines of the first and last history lines. This effectively
  skips empty lines when navigating.

Another bugfix:

Attempting to tab-complete an alias or $PATH item containing wide
characters before the first normal-width character has been entered
fails in both vi and emacs modes (see #532). The mechanism for
finding empty lines does not recognize wide non-space characters as
non-space characters, so lines with nothing but wide characters
always fail to reach the completion stage. When SHOPT_MULTIBYTE,
use !iswspace rather than !isspace to detect wide characters.
(bug introduced by a874586)

Resolves: #532
Resolves: #563
  • Loading branch information
pghvlaans authored and McDutchie committed Oct 31, 2022
1 parent 8ff5410 commit 5eeeed0
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 34 deletions.
18 changes: 18 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,24 @@ This documents significant changes in the dev branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2022-10-31:

- In vi mode, issuing the v command from a completely empty line now invokes
the full editor with an empty file instead of beeping.

- Fixed a bug in vi mode where the v command from a line with only whitespace
resulted in an "invalid range" hist error and the editor failed to open.

- Fixed a bug in emacs mode where calling the full editor saved the current
command line to the history file without a terminating newline.

- Fixed a bug in emacs and vi modes where navigating to a null or empty
history line displays an empty line onscreen.

- Fixed a bug in emacs and vi modes where attempting to tab-complete an
alias or $PATH item containing wide characters failed before the first
normal-width character had been entered.

2022-10-28:

- [1.1 change] For security reasons, type attributes of variables are no longer
Expand Down
57 changes: 45 additions & 12 deletions src/cmd/ksh93/edit/emacs.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ One line screen editor for any program
#include "history.h"
#include "edit.h"
#include "terminal.h"
#if SHOPT_MULTIBYTE
#include <wctype.h>
#endif /* SHOPT_MULTIBYTE */

#define ESH_NFIRST
#define ESH_KAPPEND
Expand Down Expand Up @@ -175,12 +178,14 @@ 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 allwhitespace(Emacs_t*, genchar*);

int ed_emacsread(void *context, int fd,char *buff,int scend, int reedit)
{
Edit_t *ed = (Edit_t*)context;
register int c;
register int i;
register int uparrow;
register genchar *out;
register int count;
register Emacs_t *ep = ed->e_emacs;
Expand Down Expand Up @@ -645,6 +650,7 @@ int ed_emacsread(void *context, int fd,char *buff,int scend, int reedit)
continue;
#endif
}
uparrow = 1;
goto common;

case cntl('O') :
Expand All @@ -671,6 +677,7 @@ int ed_emacsread(void *context, int fd,char *buff,int scend, int reedit)
}
hline = location.hist_command;
hloff = location.hist_line;
uparrow = 0;
common:
#ifdef ESH_NFIRST
location.hist_command = hline; /* save current position */
Expand All @@ -686,6 +693,13 @@ int ed_emacsread(void *context, int fd,char *buff,int scend, int reedit)
eol = genlen(out);
cur = eol;
draw(ep,UPDATE);
if(allwhitespace(ep,out))
{
if(!uparrow && hline != histlines)
ed_ungetchar(ep->ed,cntl('N'));
if(uparrow && hline != hismin)
ed_ungetchar(ep->ed,cntl('P'));
}
continue;
}
}
Expand Down Expand Up @@ -951,18 +965,7 @@ static int escape(register Emacs_t* ep,register genchar *out,int count)
case '*': /* filename expansion */
case '=': /* escape = - list all matching file names */
{
char allempty = 1;
int x;
ep->mark = cur;
for(x=0; x < cur; x++)
{
if(!isspace(out[x]))
{
allempty = 0;
break;
}
}
if(cur<1 || allempty)
if(cur<1 || allwhitespace(ep,out))
{
beep();
return(-1);
Expand Down Expand Up @@ -1233,6 +1236,21 @@ static void xcommands(register Emacs_t *ep,int count)

#ifdef ESH_BETTER
case cntl('E'): /* invoke emacs on current command */
if(eol>=0 && sh.hist_ptr)
{
if(allwhitespace(ep,drawbuff))
{
cur = 0;
eol = 1;
drawbuff[cur] = '\n';
drawbuff[eol] = '\0';
}
else
{
drawbuff[eol] = '\n';
drawbuff[eol+1] = '\0';
}
}
if(ed_fulledit(ep->ed)==-1)
beep();
else
Expand Down Expand Up @@ -1688,4 +1706,19 @@ static int _isword(register int c)
}
#endif /* SHOPT_MULTIBYTE */

static char allwhitespace(register Emacs_t *ep, register genchar *out)
{
int x;
ep->mark = cur;
for(x=0; x < cur; x++)
{
#if SHOPT_MULTIBYTE
if(!iswspace((wchar_t)out[x]))
#else
if(!isspace(out[x]))
#endif /* SHOPT_MULTIBYTE */
return(0);
}
return(1);
}
#endif /* SHOPT_ESH */
63 changes: 41 additions & 22 deletions src/cmd/ksh93/edit/vi.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@

#if SHOPT_MULTIBYTE
# include "lexstates.h"
# include <wctype.h>
# define gencpy(a,b) ed_gencpy(a,b)
# define genncpy(a,b,n) ed_genncpy(a,b,n)
# define genlen(str) ed_genlen(str)
Expand Down Expand Up @@ -174,6 +175,7 @@ typedef struct _vi_

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

static char allwhitespace(Vi_t*);
static void cursor(Vi_t*, int);
static void del_line(Vi_t*,int);
static int getcount(Vi_t*,int);
Expand Down Expand Up @@ -704,6 +706,7 @@ static int cntlmode(Vi_t *vp)
{
register int c;
register int i;
register int uparrow;
genchar tmp_u_space[MAXLINE]; /* temporary u_space */
genchar *real_u_space; /* points to real u_space */
int tmp_u_column = INVALID; /* temporary u_column */
Expand Down Expand Up @@ -859,6 +862,7 @@ static int cntlmode(Vi_t *vp)
}
save_v(vp);
cur_virt = INVALID;
uparrow = 0;
goto newhist;

case 'k': /** get previous command **/
Expand All @@ -881,6 +885,7 @@ static int cntlmode(Vi_t *vp)
}
save_v(vp);
cur_virt = INVALID;
uparrow = 1;
newhist:
if(curhline!=histmax || cur_virt==INVALID)
hist_copy((char*)virtual, MAXLINE, curhline,-1);
Expand All @@ -896,6 +901,13 @@ static int cntlmode(Vi_t *vp)
#endif /* SHOPT_MULTIBYTE */
if((last_virt=genlen(virtual)-1) >= 0 && cur_virt == INVALID)
cur_virt = 0;
if(allwhitespace(vp))
{
if(uparrow && curhline != histmin)
ed_ungetchar(vp->ed,'k');
if(!uparrow && curhline != histmax)
ed_ungetchar(vp->ed,'j');
}
break;


Expand All @@ -917,7 +929,16 @@ static int cntlmode(Vi_t *vp)

case 'v':
if(vp->repeat_set==0)
{
if(allwhitespace(vp) || cur_virt == INVALID)
{
cur_virt = 0;
last_virt = cur_virt;
refresh(vp,TRANSLATE);
virtual[last_virt++] = '\n';
}
goto vcommand;
}
/* FALLTHROUGH */

case 'G': /** goto command repeat **/
Expand Down Expand Up @@ -1485,17 +1506,7 @@ static void getline(register Vi_t* vp,register int mode)

case '\t': /** command completion **/
{
char allempty = 1;
int x;
for(x=0; x <= cur_virt; x++)
{
if(!isspace(virtual[x]))
{
allempty = 0;
break;
}
}
if(allempty)
if(allwhitespace(vp))
{
ed_ringbell();
break;
Expand Down Expand Up @@ -2533,17 +2544,7 @@ static int textmod(register Vi_t *vp,register int c, int mode)
case '\\': /** do file name completion in place **/
case '=': /** list file name expansions **/
{
char allempty = 1;
int x;
for(x=0; x <= cur_virt; x++)
{
if(!isspace(virtual[x]))
{
allempty = 0;
break;
}
}
if(cur_virt == INVALID || allempty)
if(cur_virt == INVALID || allwhitespace(vp))
return(BAD);
/* FALLTHROUGH */
save_v(vp);
Expand Down Expand Up @@ -2884,6 +2885,24 @@ static int textmod(register Vi_t *vp,register int c, int mode)
}
#endif /* SHOPT_MULTIBYTE */

/*
* determine if a line is entirely blank
*/
static char allwhitespace(register Vi_t *vp)
{
int x;
for(x=0; x <= cur_virt; x++)
{
#if SHOPT_MULTIBYTE
if(!iswspace((wchar_t)virtual[x]))
#else
if(!isspace(virtual[x]))
#endif /* SHOPT_MULTIBYTE */
return(0);
}
return(1);
}

/*
* get a character, after ^V processing
*/
Expand Down

0 comments on commit 5eeeed0

Please sign in to comment.