Re: another bug in suffix removal 3.1.5

Re: another bug in suffix removal 3.1.5

[Sven Wischnowsky]

greg@alphatech.com wrote:

> OK, I found another one:
> 
> % mkdir foo
> % mkdir foo/bar
> % bar <Control-a> <Control-k>   # IE, get "bar" in the kill ring
> % ls f <tab>
>   -> foo/ <Control-y>
>   -> foobar                     # the slash got removed when "bar" was yanked

We had some more discussion about this on the list...

No patch for this now (I have to go soon), but this looks like yet
another missing fixsuffix() problem. And this was what I meant with my 
reply: as long as we can't be sure that every time the command line is 
changed fixsuffix() is called we probably should move all the calls
into the functions that actually modify the line (might save us from
more bug reports of this kind).

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: another bug in suffix removal 3.1.5

[Peter Stephenson]

Sven Wischnowsky wrote:
> greg@alphatech.com wrote:
> 
> > OK, I found another one:
> > 
> > % mkdir foo
> > % mkdir foo/bar
> > % bar <Control-a> <Control-k>   # IE, get "bar" in the kill ring
> > % ls f <tab>
> >   -> foo/ <Control-y>
> >   -> foobar                     # the slash got removed when "bar" was yank
> ed
> 
> No patch for this now (I have to go soon), but this looks like yet
> another missing fixsuffix() problem.

Isn't it just a case of adding ZLE_KEEPSUFFIX to the flags for yank?
The following would be my suggestion.  Seems to work.

*** Src/Zle/iwidgets.list.yank	Thu Nov  5 17:23:03 1998
--- Src/Zle/iwidgets.list	Thu Nov  5 17:38:23 1998
***************
*** 145,152 ****
  "vi-open-line-below", viopenlinebelow, 0
  "vi-oper-swap-case", vioperswapcase, 0
  "vi-pound-insert", vipoundinsert, 0
! "vi-put-after", viputafter, ZLE_YANK
! "vi-put-before", viputbefore, ZLE_YANK
  "vi-quoted-insert", viquotedinsert, ZLE_MENUCMP | ZLE_KEEPSUFFIX
  "vi-repeat-change", virepeatchange, 0
  "vi-repeat-find", virepeatfind, 0
--- 145,152 ----
  "vi-open-line-below", viopenlinebelow, 0
  "vi-oper-swap-case", vioperswapcase, 0
  "vi-pound-insert", vipoundinsert, 0
! "vi-put-after", viputafter, ZLE_YANK | ZLE_KEEPSUFFIX
! "vi-put-before", viputbefore, ZLE_YANK | ZLE_KEEPSUFFIX
  "vi-quoted-insert", viquotedinsert, ZLE_MENUCMP | ZLE_KEEPSUFFIX
  "vi-repeat-change", virepeatchange, 0
  "vi-repeat-find", virepeatfind, 0
***************
*** 168,172 ****
  "what-cursor-position", whatcursorposition, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
  "where-is", whereis, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
  "which-command", processcmd, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
! "yank", yank, ZLE_YANK
! "yank-pop", yankpop, ZLE_YANK
--- 168,172 ----
  "what-cursor-position", whatcursorposition, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
  "where-is", whereis, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
  "which-command", processcmd, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
! "yank", yank, ZLE_YANK | ZLE_KEEPSUFFIX
! "yank-pop", yankpop, ZLE_YANK | ZLE_KEEPSUFFIX

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarotti 2, 56100 Pisa, Italy

Re: another bug in suffix removal 3.1.5

[Sven Wischnowsky]

Peter Stephenson wrote:

> 
> Sven Wischnowsky wrote:
> > greg@alphatech.com wrote:
> > 
> > > OK, I found another one:
> > > 
> > > % mkdir foo
> > > % mkdir foo/bar
> > > % bar <Control-a> <Control-k>   # IE, get "bar" in the kill ring
> > > % ls f <tab>
> > >   -> foo/ <Control-y>
> > >   -> foobar                     # the slash got removed when "bar" was yank
> > ed
> > 
> > No patch for this now (I have to go soon), but this looks like yet
> > another missing fixsuffix() problem.
> 
> Isn't it just a case of adding ZLE_KEEPSUFFIX to the flags for yank?
> The following would be my suggestion.  Seems to work.

Probably depends on what behavior you like if there is nothing to
yank. Without your patch, the suffix is removed which is certainly
wrong. With your patch the suffix is kept and made non-auto-removable.
Personally I would prefer zsh to leave the suffix on the line and
automatically removable in whenever a zle widget doesn't change the
line and cursor position. But maybe I'm the only one who would prefer
that.


Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: another bug in suffix removal 3.1.5

[Bart Schaefer]

On Nov 6,  8:50am, Sven Wischnowsky wrote:
} Subject: Re: another bug in suffix removal 3.1.5
}
} Peter Stephenson wrote:
} > 
} > Sven Wischnowsky wrote:
} > > greg@alphatech.com wrote:
} > > 
} > > > % bar <Control-a> <Control-k>   # IE, get "bar" in the kill ring
} > > > % ls f <tab>
} > > >   -> foo/ <Control-y>
} > > >   -> foobar                     # the slash got removed
} > > 
} > > another missing fixsuffix() problem.
} > 
} > Isn't it just a case of adding ZLE_KEEPSUFFIX to the flags for yank?
} 
} Personally I would prefer zsh to leave the suffix on the line and
} automatically removable in whenever a zle widget doesn't change the
} line and cursor position. But maybe I'm the only one who would prefer
} that.

Philosophical aside:

The 3.0.5 completion mechanism decides whether to autoremove almost
entirely based on what zle-command is next invoked.  3.1.5 is trying to
get away from that in order to make the widget set extensible, which is
why we have so many of these fixsuffix() problems.  ZLE_KEEPSUFFIX is
the standard escape back into the 3.0.5 style for widgets that haven't
been taught the new requirements yet, but it's a lot more reliable and
maintainable than sprinkling fixsuffix() around has been.

Back to the topic at hand:

This brings up a general question of what should happen on a widget
failure (either a feep, or when it simply can't do anything).  For
example,

zsh% mkdir /tmp/bar
zsh% ls /tmp/b<TAB>
zsh% ls /tmp/bar/<C-f>
zsh% ls /tmp/bar

Note that when I *tried* to move the cursor, the slash got removed,
even though the cursor couldn't actually move.  Similarly (with ^X^A
not bound to anything):

zsh% ls /tmp/b<TAB>
zsh% ls /tmp/bar/<C-xC-a>
(feep)
zsh% ls /tmp/bar

I think this argues (again) for some kind of return code from widget
functions (propagated via the standard "return" builtin for user-
defined widgets) so that they can notify the line editor when they do
nothing.  This could then be handled by zleread() in the same way that
not finding any binding at all is handled (except for the feep), which
we could then fix to match Sven's preference (which is a sensible one).

Until such time as all that has happened, I think PWS's fix is fine.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

another bug in suffix removal 3.1.5

[Greg Klanderman]

OK, I found another one:

% mkdir foo
% mkdir foo/bar
% bar <Control-a> <Control-k>   # IE, get "bar" in the kill ring
% ls f <tab>
  -> foo/ <Control-y>
  -> foobar                     # the slash got removed when "bar" was yanked



greg

(please copy on replies)