Re: PATCH: _cvs

[Sven Wischnowsky <wischnow@xxxxxxxxxxxxxxxxxxxxxxx>]

Tanaka Akira wrote:

> Also, I found a strange behaviour with `cvs history' in testing.
> 
> On 79 (or wider) column terminal:
> 
> is27e1u11% cvs history <TAB>
> file
> .cvsignore 
> ...
> option
> -?  -D  -T  -X  -a  -b  -c  -e  -f  -l  -m  -n  -o  -p  -r  -t  -u  -w  -x  -z
> 
> On 78 (or narrower) column terminal:
> 
> is27e1u11% cvs history <TAB>
> file
> .cvsignore      Doc/            README          aczsh.m4 
> .distfiles      Etc/            Src/            config.guess*
> ChangeLog       Functions/      StartupFiles/   config.sub*
> ChangeLog.3.0   INSTALL         Util/           configure.in 
> Completion/     Makefile.in     acconfig.h      install-sh*
> Config/         Misc/           aclocal.m4      mkinstalldirs*
> option
> -?  -T  -a  -c  -f  -m  -o  -r  -u  -x
> -D  -X  -b  -e  -l  -n  -p  -t  -w  -z
> 
> I suppose that the former is not intentional.

;-) It was. These display lists are a bit strange (although initially
not implemented by me -- I still tried to keep the behavior as it
was). The problem is that on a wide terminal all options fit on one
line, ,so there was no newlin in the display string. The listing code
tested that and -- with only strings without newlines -- wanted to
display them in columns. Now, the columns of all groups have the same
width (to make the display more uniform), but the one string in the
display list was rather longish and hence forced the one-column output 
for the other groups, too. If there had been at least one newline in
the display string the listing code would have treated this as a
special case and displayed it directly without taking the value for
it's widest column from it.

The patch below is a bit of a hack: it tests if there is only one
string in the display list and if so, it makes this string be
displayed as if it had a newline -- so that the other groups are 
separately put into columns.

Maybe someone can think of a better test for this sometime.


In another message:

> Z(2):akr@is27e1u11% Src/zsh -f
> is27e1u11% bindkey -e; fpath=($PWD/Completion/*(/)); autoload -U compinit; compinit -D; compdef _tst tst
> is27e1u11% _tst () { compadd -J option -X option abc def; compadd -J option -X option -y '(mno pqr)' ghi jkl } 
> is27e1u11% tst 
> option
> abc   def   
> option
> mno   pqr   
> 
> There are two groups. Is this intentional?

Again, yes, it was. Since display lists are so special with respect to 
listing, the `-y' enforces the opening of a new group, treated
completely differently and *not* with the name given with `-[JV]' in
the same call to `comp{ctl,add,gen}'.
The patch below at least makes the explanation string be added to the
group named by a `-[JV]' option, if any. But you still get different
groups, i.e. the output now looks like:

  option
  abc   def
  mno   pqr

This is a bit ugly, and I wished, the display list stuff had been
written in a way that allowed mixing the display strings with normal
matches, but because of the special treatment of newlines, this would
be very hard to implement. I wished we had two forms of this, one that 
gives only *one* display string (probably with newlines) that would be 
display unchanged, not in columns, and another form that gives a list
of strings that are like normal matches (probably even with a way to
say which of the display strings corresponds to which match -- that
would have saved us some of the work that is now done in functions
like `_display').

Bye
 Sven

diff -u os/Zle/complist.c Src/Zle/complist.c
--- os/Zle/complist.c	Wed Sep  8 16:20:32 1999
+++ Src/Zle/complist.c	Thu Sep  9 09:55:06 1999
@@ -344,7 +344,7 @@
 		nl = !!strchr(*pp++, '\n');
 
 	    pp = g->ylist;
-	    if (nl) {
+	    if (nl || !pp[1]) {
 		/* Yup, there are newlines, count lines. */
 		char *nlptr, *sptr;
 
diff -u os/Zle/zle_tricky.c Src/Zle/zle_tricky.c
--- os/Zle/zle_tricky.c	Wed Sep  8 16:20:33 1999
+++ Src/Zle/zle_tricky.c	Thu Sep  9 10:06:55 1999
@@ -4031,8 +4031,6 @@
 	    }
 	    compnmatches = mnum;
 	    compnnmatches = nmnum;
-	    if (dat->exp)
-		addexpl();
 	    if (dat->apar)
 		set_param(dat->apar, aparl);
 	    if (dat->opar)
@@ -4040,9 +4038,19 @@
 	    if (dat->dpar)
 		set_param(dat->dpar, dparl);
 	    if (dat->ylist) {
-		endcmgroup(get_user_var(dat->ylist));
-		begcmgroup("default", 0);
-	    }
+		if (dat->group) {
+		    endcmgroup(get_user_var(dat->ylist));
+		    begcmgroup(dat->group, (dat->aflags & CAF_NOSORT));
+		    if (dat->exp)
+			addexpl();
+		} else {
+		    if (dat->exp)
+			addexpl();
+		    endcmgroup(get_user_var(dat->ylist));
+		    begcmgroup("default", 0);
+		}
+	    } else if (dat->exp)
+		addexpl();
 	} LASTALLOC;
     } SWITCHBACKHEAPS;
 
@@ -6724,7 +6732,8 @@
 	    incompctlfunc = 0;
 	    uv = "reply";
 	}
-	
+	if (uv)
+	    yaptr = get_user_var(uv);
 	if ((tt = cc->explain)) {
 	    tt = dupstring(tt);
 	    if ((cc->mask & CC_EXPANDEXPL) && !parsestr(tt)) {
@@ -6732,13 +6741,16 @@
 		untokenize(tt);
 	    }
 	    expl->str = tt;
-	    addexpl();
+	    if (cc->gname) {
+		endcmgroup(yaptr);
+		begcmgroup(cc->gname, cc->mask2 & CC_NOSORT);
+		addexpl();
+	    } else {
+		addexpl();
+		endcmgroup(yaptr);
+		begcmgroup("default", 0);
+	    }
 	}
-	if (uv && (yaptr = get_user_var(uv)))
-	    endcmgroup(yaptr);
-	else
-	    endcmgroup(NULL);
-	begcmgroup("default", 0);
     } else if ((tt = cc->explain)) {
 	tt = dupstring(tt);
 	if ((cc->mask & CC_EXPANDEXPL) && !parsestr(tt)) {
@@ -8135,7 +8147,7 @@
 		nl = !!strchr(*pp++, '\n');
 
 	    pp = g->ylist;
-	    if (nl) {
+	    if (nl || !pp[1]) {
 		/* Yup, there are newlines, count lines. */
 		char *nlptr, *sptr;
 

--
Sven Wischnowsky                         wischnow@xxxxxxxxxxxxxxxxxxxxxxx