Re: Bug#190948: Violation against The Single UNIX ® Specification, Version 2

Re: Bug#190948: Violation against The Single UNIX ® Specification, Version 2

[Oliver Kiddle]

On 27 Apr, Juhapekka Tolvanen wrote:
> 
> http://www.opengroup.org/onlinepubs/007908799/xcu/chap2.html#tag_001_014_011
> 
> juhtolv@heresy:/home/juhtolv % set -o vi
> set: no such option: vi

Being an interactive related feature, this isn't so critical but it would
be better if we can handle this. Adding a special case in bin_set()
instead of actually adding an option might perhaps be sufficient to
satisfy the specification. However, on the basis that zle is a separate
module and may not even be loaded and you can't just call selectkeymap(),
I have no idea how to get this to work. And which keymap should set +o
vi select?

Oliver

RE: Bug#190948: Violation against The Single UNIX R Specification, Version 2

[Borzenkov Andrey]

> 
> On 27 Apr, Juhapekka Tolvanen wrote:
> >
> >
> http://www.opengroup.org/onlinepubs/007908799/xcu/chap2.html#tag_001_014_0
> 11
> >
> > juhtolv@heresy:/home/juhtolv % set -o vi
> > set: no such option: vi
> 
> Being an interactive related feature, this isn't so critical but it would
> be better if we can handle this. Adding a special case in bin_set()
> instead of actually adding an option might perhaps be sufficient to
> satisfy the specification. However, on the basis that zle is a separate
> module and may not even be loaded and you can't just call selectkeymap(),
> I have no idea how to get this to work. And which keymap should set +o
> vi select?
> 

========
Allow shell command-line editing using the built-in vi editor. Enabling vi
mode disables any other command-line editing mode provided as an
implementation extension. It need not be possible to set vi mode on for
certain block-mode terminals.
========

So it is extension and optional. We can just recognize option and set a flag
so Zle selects vi keymap when it starts up in shell compatibility mode. If
Zle is not loaded - just do nothing implying that  implementation extension
is not provided :)

What do you mean about set +o vi? It does not look like this form is
required:

set [-abCefmnuvx][-h][-o option][argument...]
set [+abCefmnuvx][+h][-o option][argument...]
set --[argument...]
set -[argument...]

-andrey

Re: Bug#190948: Violation against The Single UNIX ® Specification, Version 2

[Peter Stephenson]

Oliver Kiddle wrote:
> On 27 Apr, Juhapekka Tolvanen wrote:
> > 
> > http://www.opengroup.org/onlinepubs/007908799/xcu/chap2.html#tag_001_014_01
> 1
> > 
> > juhtolv@heresy:/home/juhtolv % set -o vi
> > set: no such option: vi
> 
> Being an interactive related feature, this isn't so critical but it would
> be better if we can handle this. Adding a special case in bin_set()
> instead of actually adding an option might perhaps be sufficient to
> satisfy the specification. However, on the basis that zle is a separate
> module and may not even be loaded and you can't just call selectkeymap(),
> I have no idea how to get this to work. And which keymap should set +o
> vi select?

Exactly, it's completely meaningless in zsh.  It is ridiculous for the
standard to specify how the shell handles line editing, it implies far
too much about the architecture, and I would suggest it actively
encourages poor component design.  Who says a shell *has* to have a
version of vi built into it, for goodness sake?

Although in principle we could handle it I would be quite happy to make
a point of it being a dummy or printing an error message, with a
rationale in the documentation.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: Bug#190948: Violation against The Single UNIX ? Specification, Version 2

[Zefram]

Peter Stephenson wrote:
>Exactly, it's completely meaningless in zsh.  It is ridiculous for the
>standard to specify how the shell handles line editing,

Yet the standard does.  A vi command line editing mode is defined
by POSIX.  An Emacs mode would also have been in the standard, but the
committee couldn't reach agreement on the details.  (ISTR seeing RMS
named in the credits, I wonder what really went on.)  I think Zle's vi
mode behaviour is incompatible with POSIX on some points, but I really
don't care, because we did better there.

I think we should, at least, arrange that "set -o vi" in an interactive
zsh with zle loaded has the effect of "bindkey -v".  That perserves
the intent of the standard -- that "set -o vi" is the portable way for
a user to select vi-style command line editing -- and we don't have to
go ridiculously out of our way to achieve it.

I suggest having a hook to execute on "set -o vi"; Zle would hook this
when it is loaded, just like the other things it already hooks.  To be
nice to everyone that knows that, despite POSIX, interactive shells do
actually have an Emacs mode too, it would be good to hook "set -o emacs"
as well.  Only these two are required.

-zefram

Re: Bug#190948: Violation against The Single UNIX ? Specification, Version 2

[Peter Stephenson]

Zefram wrote:
> I suggest having a hook to execute on "set -o vi"; Zle would hook this
> when it is loaded, just like the other things it already hooks.  To be
> nice to everyone that knows that, despite POSIX, interactive shells do
> actually have an Emacs mode too, it would be good to hook "set -o emacs"
> as well.  Only these two are required.
> 
> -zefram

Gives us something to spit at.

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.27
diff -u -r1.27 options.yo
--- Doc/Zsh/options.yo	4 Dec 2002 13:57:52 -0000	1.27
+++ Doc/Zsh/options.yo	14 May 2003 14:10:42 -0000
@@ -367,6 +367,14 @@
 for examining spelling mistakes for the tt(CORRECT) and tt(CORRECT_ALL)
 options and the tt(spell-word) editor command.
 )
+pindex(EMACS)
+item(tt(EMACS))(
+If ZLE is loaded, turning on this option has the equivalent effect
+of `tt(bindkey -e)'.  In addition, the VI option is unset.
+Turning it off has no effect.  The option setting is
+not guaranteed to reflect the current keymap.  This option is
+provided for compatibility; tt(bindkey) is the recommended interface.
+)
 pindex(EQUALS)
 cindex(filename expansion, =)
 item(tt(EQUALS) <Z>)(
@@ -1192,6 +1200,14 @@
 cindex(input, tracing)
 item(tt(VERBOSE) (tt(-v), ksh: tt(-v)))(
 Print shell input lines as they are read.
+)
+pindex(VI)
+item(tt(VI))(
+If ZLE is loaded, turning on this option has the equivalent effect
+of `tt(bindkey -v)'.  In addition, the EMACS option is unset.
+Turning it off has no effect.  The option setting is
+not guaranteed to reflect the current keymap.  This option is
+provided for compatibility; tt(bindkey) is the recommended interface.
 )
 pindex(XTRACE)
 cindex(tracing, of commands)
Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.32
diff -u -r1.32 init.c
--- Src/init.c	23 Apr 2003 18:04:48 -0000	1.32
+++ Src/init.c	14 May 2003 14:10:42 -0000
@@ -1112,6 +1112,8 @@
 mod_export ZleVoidIntFn spaceinlineptr = noop_function_int;
 /**/
 mod_export ZleReadFn zlereadptr = autoload_zleread;
+/**/
+mod_export ZleVoidIntFn zlesetkeymapptr = noop_function_int;
 
 #else /* !LINKED_XMOD_zshQszle */
 
@@ -1124,6 +1126,7 @@
 mod_export ZleReadFn zlereadptr = fallback_zleread;
 # endif /* !UNLINKED_XMOD_zshQszle */
 
+mod_export ZleVoidIntFn zlesetkeymapptr = noop_function_int;
 #endif /* !LINKED_XMOD_zshQszle */
 
 /**/
Index: Src/options.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/options.c,v
retrieving revision 1.14
diff -u -r1.14 options.c
--- Src/options.c	4 Dec 2002 13:57:51 -0000	1.14
+++ Src/options.c	14 May 2003 14:10:42 -0000
@@ -107,6 +107,7 @@
 {NULL, "cshjunkiequotes",     OPT_EMULATE|OPT_CSH,	 CSHJUNKIEQUOTES},
 {NULL, "cshnullcmd",	      OPT_EMULATE|OPT_CSH,	 CSHNULLCMD},
 {NULL, "cshnullglob",	      OPT_EMULATE|OPT_CSH,	 CSHNULLGLOB},
+{NULL, "emacs",		      0,			 EMACSMODE},
 {NULL, "equals",	      OPT_EMULATE|OPT_ZSH,	 EQUALS},
 {NULL, "errexit",	      OPT_EMULATE,		 ERREXIT},
 {NULL, "errreturn",	      OPT_EMULATE,		 ERRRETURN},
@@ -204,6 +205,7 @@
 {NULL, "typesetsilent",	      OPT_EMULATE|OPT_BOURNE,	 TYPESETSILENT},
 {NULL, "unset",		      OPT_EMULATE|OPT_BSHELL,	 UNSET},
 {NULL, "verbose",	      0,			 VERBOSE},
+{NULL, "vi",		      0,			 VIMODE},
 {NULL, "xtrace",	      0,			 XTRACE},
 {NULL, "zle",		      OPT_SPECIAL,		 USEZLE},
 {NULL, "braceexpand",	      OPT_ALIAS, /* ksh/bash */	 -IGNOREBRACES},
@@ -679,6 +681,9 @@
     } else if(optno == CDABLEVARS && value) {
 	    return -1;
 #endif /* GETPWNAM_FAKED */
+    } else if ((optno == EMACSMODE || optno == VIMODE) && value) {
+	(*zlesetkeymapptr)(optno);
+	opts[(optno == EMACSMODE) ? VIMODE : EMACSMODE] = 0;
     }
     opts[optno] = value;
     if (optno == BANGHIST || optno == SHINSTDIN)
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.46
diff -u -r1.46 zsh.h
--- Src/zsh.h	2 May 2003 10:25:33 -0000	1.46
+++ Src/zsh.h	14 May 2003 14:10:42 -0000
@@ -1418,6 +1418,7 @@
     CSHJUNKIEQUOTES,
     CSHNULLCMD,
     CSHNULLGLOB,
+    EMACSMODE,
     EQUALS,
     ERREXIT,
     ERRRETURN,
@@ -1515,6 +1516,7 @@
     TYPESETSILENT,
     UNSET,
     VERBOSE,
+    VIMODE,
     XTRACE,
     USEZLE,
     DVORAK,
Index: Src/Zle/zle_keymap.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_keymap.c,v
retrieving revision 1.9
diff -u -r1.9 zle_keymap.c
--- Src/Zle/zle_keymap.c	27 Aug 2002 21:10:34 -0000	1.9
+++ Src/Zle/zle_keymap.c	14 May 2003 14:10:43 -0000
@@ -1370,3 +1370,13 @@
 	func = lastnamed;
     return func;
 }
+
+/**/
+mod_export void
+zlesetkeymap(int mode)
+{
+    Keymap km = openkeymap((mode == VIMODE) ? "viins" : "emacs");
+    if (!km)
+	return;
+    linkkeymap(km, "main", 0);
+}
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.31
diff -u -r1.31 zle_main.c
--- Src/Zle/zle_main.c	17 Feb 2003 14:07:12 -0000	1.31
+++ Src/Zle/zle_main.c	14 May 2003 14:10:43 -0000
@@ -1363,6 +1363,7 @@
     refreshptr = zrefresh;
     spaceinlineptr = spaceinline;
     zlereadptr = zleread;
+    zlesetkeymapptr = zlesetkeymap;
 
     getkeyptr = getkey;
 
@@ -1446,6 +1447,7 @@
     refreshptr = noop_function;
     spaceinlineptr = noop_function_int;
     zlereadptr = fallback_zleread;
+    zlesetkeymapptr= noop_function_int;
 
     getkeyptr = NULL;
 
Index: Src/Zle/zle_vi.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_vi.c,v
retrieving revision 1.1.1.9
diff -u -r1.1.1.9 zle_vi.c
--- Src/Zle/zle_vi.c	3 Jul 1999 13:18:04 -0000	1.1.1.9
+++ Src/Zle/zle_vi.c	14 May 2003 14:10:43 -0000
@@ -89,7 +89,7 @@
 startvitext(int im)
 {
     startvichange(im);
-    selectkeymap("main", 1);
+    selectkeymap("viins", 1);
     undoing = 0;
     viinsbegin = cs;
 }
@@ -98,7 +98,7 @@
 int
 vigetkey(void)
 {
-    Keymap mn = openkeymap("main");
+    Keymap mn = openkeymap("viins");
     char m[3], *str;
     Thingy cmd;
 
@@ -351,7 +351,7 @@
     if ((c2 = getvirange(1)) != -1) {
 	ret = 0;
 	forekill(c2 - cs, 0);
-	selectkeymap("main", 1);
+	selectkeymap("viins", 1);
 	viinsbegin = cs;
 	undoing = 0;
     }

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: Bug#190948: Violation against The Single UNIX ? Specification, Version 2

[Peter Stephenson]

Peter Stephenson wrote:
> Index: Src/Zle/zle_vi.c

Whoops, ignore this part; that was some irrelevant hacking which won't
be committed.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: Bug#190948: Violation against The Single UNIX ? Specification, Version 2

[Peter Stephenson]

Peter Stephenson wrote:
> +pindex(VI)
> +item(tt(VI))(
> +If ZLE is loaded, turning on this option has the equivalent effect
> +of `tt(bindkey -v)'.  In addition, the EMACS option is unset.
> +Turning it off has no effect.  The option setting is
> +not guaranteed to reflect the current keymap.  This option is
> +provided for compatibility; tt(bindkey) is the recommended interface.
>  )

Sigh.  Doesn't autoload zle, so `set -o vi' before any zle configuration
in .zshrc doesn't have any effect.  Actually covered by the weasel words
at the start of the manual entry, but I think this is one of those
occasions where implementing the feature is easier than explaining every
time why it doesn't work the way that's expected.

By the way, this neighbourhood of init.c contains the comment:

/* ZLE entry point pointers.  They are defined here because the initial *
 * values depend on whether ZLE is linked in or not -- if it is, we     *
 * avoid wasting space with the fallback functions.  No other source    *
 * file needs to know which modules are linked in.                      */

Except we don't avoid wasting space with the fallback functions, they
are defined unconditionally.  Anyone remember what's happening?


Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.33
diff -u -r1.33 init.c
--- Src/init.c	14 May 2003 15:09:14 -0000	1.33
+++ Src/init.c	15 May 2003 09:12:10 -0000
@@ -1122,11 +1122,12 @@
 mod_export ZleVoidIntFn spaceinlineptr = noop_function_int;
 # ifdef UNLINKED_XMOD_zshQszle
 mod_export ZleReadFn zlereadptr = autoload_zleread;
+mod_export ZleVoidIntFn zlesetkeymapptr = autoload_zlesetkeymap;
 # else /* !UNLINKED_XMOD_zshQszle */
 mod_export ZleReadFn zlereadptr = fallback_zleread;
+mod_export ZleVoidIntFn zlesetkeymapptr = noop_function_int;
 # endif /* !UNLINKED_XMOD_zshQszle */
 
-mod_export ZleVoidIntFn zlesetkeymapptr = noop_function_int;
 #endif /* !LINKED_XMOD_zshQszle */
 
 /**/
@@ -1152,6 +1153,16 @@
 
     return (unsigned char *)shingetline();
 }
+
+/**/
+static void
+autoload_zlesetkeymap(int mode)
+{
+    zlesetkeymapptr = noop_function_int;
+    load_module("zsh/zle");
+    (*zlesetkeymapptr)(mode);
+}
+
 
 /* compctl entry point pointers.  Similar to the ZLE ones. */
 

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: Bug#190948: Violation against The Single UNIX ? Specification, Version 2

[Zefram]

Peter Stephenson wrote:
>/* ZLE entry point pointers.  They are defined here because the initial *
> * values depend on whether ZLE is linked in or not -- if it is, we     *
> * avoid wasting space with the fallback functions.  No other source    *
> * file needs to know which modules are linked in.                      */
>
>Except we don't avoid wasting space with the fallback functions, they
>are defined unconditionally.  Anyone remember what's happening?

I wrote that comment.  The intent was as described, and I have a mental
image of the fallback functions being in an appropriate conditional
section.  I remember adding mechanism to the build process to make that
information available to the preprocessor.  If the fallback functions
are no longer conditionally compiled, it'll be an accidental change,
unless I got it wrong to start with.

-zefram

Re: Bug#190948: Violation against The Single UNIX ? Specification, Version 2

[Peter Stephenson]

Zefram wrote:
> Peter Stephenson wrote:
> >/* ZLE entry point pointers.  They are defined here because the initial *
> > * values depend on whether ZLE is linked in or not -- if it is, we     *
> > * avoid wasting space with the fallback functions.  No other source    *
> > * file needs to know which modules are linked in.                      */
> 
> I wrote that comment.  The intent was as described, and I have a mental
> image of the fallback functions being in an appropriate conditional
> section.

The autoload version of zleread (now) sets the pointer to the fallback
version in case it fails.  So the only gain would seem to be you can
eliminate the autoload versions if zle is already linked.  However, the
standard mechanism at the moment is to use the dynamic link routines
even if zle is built into the shell (or at least zmodload acts the same
however the library is linked, I haven't checked further).  Plus if zle
is built into the shell removing the autoload functions to save space is
rather laughable.  So I don't think I'll change anything, except maybe
update the comment.

OK, it's back to Bluetooth scatter mode...

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************