Re: PATCH: documentation on keymap selection

Re: PATCH: documentation on keymap selection

[Peter Stephenson]

Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> It doesn't address the issue that "bindkey -lL" lies about aliased
> keymaps.

This does.  It's minor, but it's a real gap --- in addition to not
knowing which keymaps are aliases for others, you can't even find out
trivially whether you're "in emacs mode" or "in vi mode".  The inverted
commas are because technically you never are, you just have some keymap
or other linked to main which happens to have a set of emacs-like or
vi-like bindings.  However, by keeping track of names of keymaps that
aren't "main" we can give a consistent account of what's happening such
that in the normal cases it does look exactly like you're in emacs or vi
mode.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.84
diff -p -u -r1.84 zle.yo
--- Doc/Zsh/zle.yo	1 Sep 2010 16:39:32 -0000	1.84
+++ Doc/Zsh/zle.yo	3 Sep 2010 21:53:07 -0000
@@ -175,7 +181,8 @@ startitem()
 item(tt(-l))(
 List all existing keymap names.  If the tt(-L)
 option is also used, list in the form of tt(bindkey)
-commands to create the keymaps.
+commands to create the keymaps; this combination also shows
+which keymap is linked to `tt(main)', if any.
 )
 item(tt(-d))(
 Delete all existing keymaps and reset to the default state.
Index: Src/Zle/zle_keymap.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_keymap.c,v
retrieving revision 1.32
diff -p -u -r1.32 zle_keymap.c
--- Src/Zle/zle_keymap.c	28 Jan 2009 09:32:01 -0000	1.32
+++ Src/Zle/zle_keymap.c	3 Sep 2010 21:53:07 -0000
@@ -58,11 +58,24 @@ struct keymapname {
     Keymap keymap;	/* the keymap itsef */
 };
 
+/* Can't be deleted (.safe) */
 #define KMN_IMMORTAL (1<<1)
 
 struct keymap {
     Thingy first[256];	/* base binding of each character */
     HashTable multi;	/* multi-character bindings */
+    /*
+     * The "real" name of this keymap.
+     * For an aliased keymap, this is the first name to be defined.
+     * If this is deleted but there are other names we randomly pick another
+     * one, avoiding the name "main".  The principal use
+     * for this is to make it clear what "main" is aliased to.
+     *
+     * If "main" is the only name for this map, this will be NULL.
+     * That's fine, there's no alias.  We'll pick a primary if we
+     * alias "main" again.
+     */
+    KeymapName primary;
     int flags;		/* various flags (see below) */
     int rc;		/* reference count */
 };
@@ -162,6 +175,70 @@ makekeymapnamnode(Keymap keymap)
     return kmn;
 }
 
+
+/*
+ * Reference a keymap from a keymapname.
+ * Used when linking keymaps.  This includes the first link to a
+ * newly created keymap.
+ */
+
+static void
+refkeymap_by_name(KeymapName kmn)
+{
+    refkeymap(kmn->keymap);
+    if (!kmn->keymap->primary && strcmp(kmn->nam, "main") != 0)
+	kmn->keymap->primary = kmn;
+}
+
+/*
+ * Communication to keymap scanner when looking for a new primary name.
+ */
+static Keymap km_rename_me;
+
+/* Find a new primary name for a keymap.  See below. */
+
+static void
+scanprimaryname(HashNode hn, int ignored)
+{
+    KeymapName n = (KeymapName) hn;
+
+    (void)ignored;
+
+    /* Check if we've already found a new primary name. */
+    if (km_rename_me->primary)
+	return;
+    /* Don't use "main". */
+    if (!strcmp(n->nam, "main"))
+	return;
+    if (n->keymap == km_rename_me)
+	km_rename_me->primary = n;
+}
+
+/*
+ * Unreference a keymap from a keymapname.
+ * Used when unlinking keymaps to ensure there is still a primary
+ * name for the keymap, unless it is an unaliased "main".
+ */
+static void
+unrefkeymap_by_name(KeymapName kmname)
+{
+    Keymap km = kmname->keymap;
+    if (unrefkeymap(km) && km->primary == kmname) {
+	/*
+	 * The primary name for the keymap has gone,
+	 * but the keymap is still referred to; find a new primary name
+	 * for it.  Sort the keymap to make the result deterministic.
+	 */
+	/* Set the primary name to NULL so we can check if we've found one */
+	km->primary = NULL;
+	km_rename_me = km;
+	scanhashtable(keymapnamtab, 1, 0, 0, scanprimaryname, 0);
+	/* Just for neatness */
+	km_rename_me = NULL;
+    }
+}
+
+
 /**/
 static void
 freekeymapnamnode(HashNode hn)
@@ -169,7 +246,7 @@ freekeymapnamnode(HashNode hn)
     KeymapName kmn = (KeymapName) hn;
 
     zsfree(kmn->nam);
-    unrefkeymap(kmn->keymap);
+    unrefkeymap_by_name(kmn);
     zfree(kmn, sizeof(*kmn));
 }
 
@@ -354,7 +431,7 @@ linkkeymap(Keymap km, char *name, int im
 	    return 1;
 	if(n->keymap == km)
 	    return 0;
-	unrefkeymap(n->keymap);
+	unrefkeymap_by_name(n);
 	n->keymap = km;
     } else {
 	n = makekeymapnamnode(km);
@@ -362,21 +439,29 @@ linkkeymap(Keymap km, char *name, int im
 	    n->flags |= KMN_IMMORTAL;
 	keymapnamtab->addnode(keymapnamtab, ztrdup(name), n);
     }
-    refkeymap(km);
+    refkeymap_by_name(n);
     return 0;
 }
 
 /**/
-void refkeymap(Keymap km)
+void
+refkeymap(Keymap km)
 {
     km->rc++;
 }
 
+/* Unreference keymap, returning new reference count, 0 if deleted */
+
 /**/
-void unrefkeymap(Keymap km)
+int
+unrefkeymap(Keymap km)
 {
-    if (!--km->rc)
+    if (!--km->rc) {
 	deletekeymap(km);
+	return 0;
+    }
+
+    return km->rc;
 }
 
 /* Select a keymap as the current ZLE keymap.  Can optionally fall back *
@@ -734,10 +819,21 @@ scanlistmaps(HashNode hn, int list)
 {
     KeymapName n = (KeymapName) hn;
 
-    if(list) {
-	fputs("bindkey -N ", stdout);
-	if(n->nam[0] == '-')
-	    fputs("-- ", stdout);
+    if (list) {
+	Keymap km = n->keymap;
+	fputs("bindkey -", stdout);
+	if (km->primary && km->primary != n) {
+	    KeymapName pn = km->primary;
+	    fputs("A ", stdout);
+	    if (pn->nam[0] == '-')
+		fputs("-- ", stdout);
+	    quotedzputs(pn->nam, stdout);
+	    fputc(' ', stdout);
+	} else {
+	    fputs("N ", stdout);
+	    if(n->nam[0] == '-')
+		fputs("-- ", stdout);
+	}
 	quotedzputs(n->nam, stdout);
     } else
 	nicezputs(n->nam, stdout);

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PATCH: documentation on keymap selection

[Peter Stephenson]

On Fri, 3 Sep 2010 23:06:10 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > It doesn't address the issue that "bindkey -lL" lies about aliased
> > keymaps.
> 
> This does.

While I'm at it, this seems a natural extension.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.85
diff -p -u -r1.85 zle.yo
--- Doc/Zsh/zle.yo	5 Sep 2010 19:24:44 -0000	1.85
+++ Doc/Zsh/zle.yo	5 Sep 2010 19:39:50 -0000
@@ -139,7 +139,7 @@ cindex(rebinding keys)
 cindex(keys, binding)
 cindex(binding keys)
 cindex(keymaps)
-xitem(tt(bindkey) [ var(options) ] tt(-l))
+xitem(tt(bindkey) [ var(options) ] tt(-l) [ tt(-l) ] [ var(keymap) ... ])
 xitem(tt(bindkey) [ var(options) ] tt(-d))
 xitem(tt(bindkey) [ var(options) ] tt(-D) var(keymap) ...)
 xitem(tt(bindkey) [ var(options) ] tt(-A) var(old-keymap new-keymap))
@@ -179,10 +179,13 @@ selected, namely:
 
 startitem()
 item(tt(-l))(
-List all existing keymap names.  If the tt(-L)
-option is also used, list in the form of tt(bindkey)
-commands to create the keymaps; this combination also shows
-which keymap is linked to `tt(main)', if any.
+List all existing keymap names; if any arguments are given, list just
+those keymaps.
+
+If the tt(-L) option is also used, list in the form of tt(bindkey)
+commands to create or link the keymaps.  `tt(bindkey -lL
+main)' shows which keymap is linked to `tt(main)', if any, and hence if
+the standard emacs or vi emulation is in effect.
 )
 item(tt(-d))(
 Delete all existing keymaps and reset to the default state.
Index: Src/Zle/zle_keymap.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_keymap.c,v
retrieving revision 1.33
diff -p -u -r1.33 zle_keymap.c
--- Src/Zle/zle_keymap.c	5 Sep 2010 19:24:49 -0000	1.33
+++ Src/Zle/zle_keymap.c	5 Sep 2010 19:39:50 -0000
@@ -724,7 +724,7 @@ bin_bindkey(char *name, char **argv, Opt
 	int (*func) _((char *, char *, Keymap, char **, Options, char));
 	int min, max;
     } const opns[] = {
-	{ 'l', 0, bin_bindkey_lsmaps, 0,  0 },
+	{ 'l', 0, bin_bindkey_lsmaps, 0,  -1 },
 	{ 'd', 0, bin_bindkey_delall, 0,  0 },
 	{ 'D', 0, bin_bindkey_del,    1, -1 },
 	{ 'A', 0, bin_bindkey_link,   2,  2 },
@@ -807,10 +807,24 @@ bin_bindkey(char *name, char **argv, Opt
 
 /**/
 static int
-bin_bindkey_lsmaps(UNUSED(char *name), UNUSED(char *kmname), UNUSED(Keymap km), UNUSED(char **argv), Options ops, UNUSED(char func))
+bin_bindkey_lsmaps(char *name, UNUSED(char *kmname), UNUSED(Keymap km), char **argv, Options ops, UNUSED(char func))
 {
-    scanhashtable(keymapnamtab, 1, 0, 0, scanlistmaps, OPT_ISSET(ops,'L'));
-    return 0;
+    int ret = 0;
+    if (*argv) {
+	for (; *argv; argv++) {
+	    KeymapName kmn = (KeymapName)
+		keymapnamtab->getnode(keymapnamtab, *argv);
+	    if (!kmn) {
+		zwarnnam(name, "no such keymap: `%s'", *argv);
+		ret = 1;
+	    } else {
+		scanlistmaps((HashNode)kmn, OPT_ISSET(ops,'L'));
+	    }
+	}
+    } else {
+	scanhashtable(keymapnamtab, 1, 0, 0, scanlistmaps, OPT_ISSET(ops,'L'));
+    }
+    return ret;
 }
 
 /**/

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PATCH: documentation on keymap selection

[Bart Schaefer]

On Sep 5,  8:40pm, Peter Stephenson wrote:
}
} While I'm at it, this seems a natural extension.
} 
} Index: Doc/Zsh/zle.yo
} +xitem(tt(bindkey) [ var(options) ] tt(-l) [ tt(-l) ] [ var(keymap) ... ])

Yes, I have often wondered why it wasn't done that way in the first place.

Re: PATCH: documentation on keymap selection

[Bart Schaefer]

On Sep 5,  8:40pm, Peter Stephenson wrote:
} Subject: Re: PATCH: documentation on keymap selection
}
} On Fri, 3 Sep 2010 23:06:10 +0100
} Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
} > Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
} > > It doesn't address the issue that "bindkey -lL" lies about aliased
} > > keymaps.
} > 
} > This does.
} 
} While I'm at it, this seems a natural extension.
} 
} +List all existing keymap names; if any arguments are given, list just
} +those keymaps.
} +
} +If the tt(-L) option is also used, list in the form of tt(bindkey)
} +commands to create or link the keymaps.

I just tried this out, and got this:

schaefer<501> bindkey -lL
bindkey -N .safe
bindkey -N command
bindkey -N emacs
bindkey -N isearch
bindkey -N listscroll
bindkey -A emacs main
bindkey -N menuselect
bindkey -N vicmd
bindkey -N viins

Is it really correct to include "bindkey -N .safe" in that output?  If
I turn around and feed that back to the shell again, I get

schaefer<502> bindkey -N .safe
bindkey: keymap name `.safe' is protected

Perhaps the .safe keymap should be treated as invisible for this.

Re: PATCH: documentation on keymap selection

[Bart Schaefer]

On Sep 7,  9:58am, Bart Schaefer wrote:
}
} schaefer<502> bindkey -N .safe
} bindkey: keymap name `.safe' is protected
} 
} Perhaps the .safe keymap should be treated as invisible for this.

And yes, I should ack that this behavior has NOT been changed/added by the
patch in question; all this patch did was add the "bindkey -A emacs main".

Re: PATCH: documentation on keymap selection

[Peter Stephenson]

On Tue, 07 Sep 2010 09:58:48 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> schaefer<501> bindkey -lL
> bindkey -N .safe
> bindkey -N command
> bindkey -N emacs
> bindkey -N isearch
> bindkey -N listscroll
> bindkey -A emacs main
> bindkey -N menuselect
> bindkey -N vicmd
> bindkey -N viins
> 
> Is it really correct to include "bindkey -N .safe" in that output?

Hmm, that's definitely a good point.

If we simply omit it the output is no longer printing all the keymaps, as
it used to.  Of course, bindkey -l does that, and you can argue bindkey -lL
needs to be useful, which is after all the aim of the previous change.
Given the new behaviour is incompatible with the old, and the old was a
lie, maybe that is the right thing to do.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.87
diff -p -u -r1.87 zle.yo
--- Doc/Zsh/zle.yo	6 Sep 2010 08:50:05 -0000	1.87
+++ Doc/Zsh/zle.yo	7 Sep 2010 17:54:10 -0000
@@ -185,7 +185,10 @@ those keymaps.
 If the tt(-L) option is also used, list in the form of tt(bindkey)
 commands to create or link the keymaps.  `tt(bindkey -lL
 main)' shows which keymap is linked to `tt(main)', if any, and hence if
-the standard emacs or vi emulation is in effect.
+the standard emacs or vi emulation is in effect.  This option does
+not show the tt(.safe) keymap because it cannot be created in that
+fashion; however, neither is `tt(bindkey -lL .safe)' reported as an
+error, it simply outputs nothing.
 )
 item(tt(-d))(
 Delete all existing keymaps and reset to the default state.
Index: Src/Zle/zle_keymap.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_keymap.c,v
retrieving revision 1.34
diff -p -u -r1.34 zle_keymap.c
--- Src/Zle/zle_keymap.c	5 Sep 2010 20:39:09 -0000	1.34
+++ Src/Zle/zle_keymap.c	7 Sep 2010 17:54:10 -0000
@@ -829,12 +829,18 @@ bin_bindkey_lsmaps(char *name, UNUSED(ch
 
 /**/
 static void
-scanlistmaps(HashNode hn, int list)
+scanlistmaps(HashNode hn, int list_verbose)
 {
     KeymapName n = (KeymapName) hn;
 
-    if (list) {
+    if (list_verbose) {
 	Keymap km = n->keymap;
+	/*
+	 * Don't list ".safe" as a bindkey command; we can't
+	 * actually create it that way.
+	 */
+	if (!strcmp(n->nam, ".safe"))
+	    return;
 	fputs("bindkey -", stdout);
 	if (km->primary && km->primary != n) {
 	    KeymapName pn = km->primary;

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

PATCH: documentation on keymap selection

[Peter Stephenson]

This is intended to improve the documentation on keymaps I noted a
couple of weeks ago.

It doesn't address the issue that "bindkey -lL" lies about aliased
keymaps.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.84
diff -p -u -r1.84 zle.yo
--- Doc/Zsh/zle.yo	1 Sep 2010 16:39:32 -0000	1.84
+++ Doc/Zsh/zle.yo	3 Sep 2010 20:44:28 -0000
@@ -149,21 +149,27 @@ xitem(tt(bindkey) [ var(options) ] tt(-r
 xitem(tt(bindkey) [ var(options) ] tt(-s) var(in-string out-string) ...)
 xitem(tt(bindkey) [ var(options) ] var(in-string command) ...)
 item(tt(bindkey) [ var(options) ] [ var(in-string) ])(
-tt(bindkey)'s options can be divided into three categories: keymap selection,
-operation selection, and others.  The keymap selection options are:
+tt(bindkey)'s options can be divided into three categories: keymap
+selection for the current command, operation selection, and others.  The
+keymap selection options are:
 
 startitem()
 item(tt(-e))(
-Selects keymap `tt(emacs)', and also links it to `tt(main)'.
+Selects keymap `tt(emacs)' for any operations by the current command,
+and also links `tt(emacs)' to `tt(main)' so that it is selected by
+default the next time the editor starts.
 )
 item(tt(-v))(
-Selects keymap `tt(viins)', and also links it to `tt(main)'.
+Selects keymap `tt(viins)' for any operations by the current command,
+and also links `tt(viins)' to `tt(main)' so that it is selected by default
+the next time the editor starts.
 )
 item(tt(-a))(
-Selects keymap `tt(vicmd)'.
+Selects keymap `tt(vicmd)' for any operations by the current command.
 )
 item(tt(-M) var(keymap))(
-The var(keymap) specifies a keymap name.
+The var(keymap) specifies a keymap name that is selected for any
+operations by the current command.
 )
 enditem()
 
@@ -1207,11 +1213,10 @@ mini-buffer.
 )
 enditem()
 
-Any multi-character string that is not bound to one of the above functions
-will beep and interrupt the search, leaving the last found line in the
-buffer. Any single character that is not bound to one of the above
-functions, or tt(self-insert) or tt(self-insert-unmeta), will have the same
-effect but the function will be executed.
+Any character that is not bound to one of the above functions, or
+tt(self-insert) or tt(self-insert-unmeta), will cause the mode to be
+exited.  The character is then looked up and executed in the keymap in
+effect at that point.
 
 When called from a widget function by the tt(zle) command, the incremental
 search commands can take a string argument.  This will be treated as a


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/