* Bug somewhere in verbose output for completion listing @ 2013-10-03 13:00 Peter Stephenson 2013-10-03 13:32 ` Peter Stephenson 2013-10-03 14:50 ` Bart Schaefer 0 siblings, 2 replies; 9+ messages in thread From: Peter Stephenson @ 2013-10-03 13:00 UTC (permalink / raw) To: Zsh Hackers' List I expect this to be greeted with something like the cough from the back of the room when Krusty the Clown tells a joke --- I can't reproduce this reliably at the moment --- but something determining output width in completion listings (I'm using the zsh/complist module) isn't being reset properly. After a while, the following: _wtf () { local stuff stuff=('SpoobleOne:2012/11/02 Description of SpoobleOne' 'SpoobleTwo:2013/02/13 Description of SpoobleTwo') _describe -t stuff 'Stuff to complete.' stuff } compdef _wtf wtf starts producing completion listings in verbose mode (the default) with zstyle ':completion:*' list-separator '#' (hence the "#" below) that look like Completing Stuff to complete. SpoobleOne # SpoobleTwo # From that point on this happens every time. In case they get wrapped, there are only three lines there, the second and thrid long with lots of spaces between the completion and what should be the description, which is missing. This doesn't happen when I first start the shell; I get the expected Completing Stuff to complete. SpoobleOne # 2012/11/02 Description of SpoobleOne SpoobleTwo # 2013/02/13 Description of SpoobleTwo I think some extra long output from _describe or similar is causing it to go haywire thereafter. In my case that seems to be to do with long completions generated by _perforce, but it's unlikely to be specific to that. I suspect some static in complist isn't being reset: I have a memory the logic is a bit tortuous. If I find a moment I'll try to make it more reproducible (although simply attaching to a running shell might be good enough to track it down). pws ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 13:00 Bug somewhere in verbose output for completion listing Peter Stephenson @ 2013-10-03 13:32 ` Peter Stephenson 2013-10-03 22:54 ` Bart Schaefer 2013-10-03 14:50 ` Bart Schaefer 1 sibling, 1 reply; 9+ messages in thread From: Peter Stephenson @ 2013-10-03 13:32 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 03 Oct 2013 14:00:25 +0100 Peter Stephenson <p.stephenson@samsung.com> wrote: > If I find a moment I'll try to make it more reproducible (although > simply attaching to a running shell might be good enough to track it > down). Appears to be trivial to reproduce in the obvious manner, by generating a single long completion. autoload -Uz compinit compinit _wtf2 () { local stuff stuff=('SpoobleOneVariantWithAVeryLongCompletionThatCouldCauseAllSortsOfMayhemAfterAllWhoKnowsWhatEvilLurksEtc:2012/11/02 Description of SpoobleOne' 'SpoobleTwo:2013/02/13 Description of SpoobleTwo') _describe -t stuff 'Stuff to complete.' stuff } compdef _wtf2 wtf2 _wtf () { local stuff stuff=('SpoobleOne:2012/11/02 Description of SpoobleOne' 'SpoobleTwo:2013/02/13 Description of SpoobleTwo') _describe -t stuff 'Stuff to complete.' stuff } compdef _wtf wtf Complete after wtf2 as far as getting a listing (the separator is the default "--" instead of the "#" in the previous posting). This is as easy as typing "wtf2 ^D". It already looks a bit screwy at this point as the comment is missing even though the separator is present. Then whenever you get a listing after wtf you see the problem. pws ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 13:32 ` Peter Stephenson @ 2013-10-03 22:54 ` Bart Schaefer 0 siblings, 0 replies; 9+ messages in thread From: Bart Schaefer @ 2013-10-03 22:54 UTC (permalink / raw) To: Zsh Hackers' List Already committed 31781. A bit additional: On Oct 3, 2:32pm, Peter Stephenson wrote: } } Complete after wtf2 as far as getting a listing (the separator is the } default "--" instead of the "#" in the previous posting). This is as } easy as typing "wtf2 ^D". It already looks a bit screwy at this point } as the comment is missing even though the separator is present. The seperator was being inserted into the string before the width was compared to zterm_columns. Most of the below is just re-indentation, but because the listing already has to deal with completions so wide that the line has wrapped, there doesn't seem to be any reason not to use the space on the last additional line to tack on the description. What this means is that if you have a long completion match that almost exactly fills the screen, you'll get all the matches with none of the descriptions. If the longest matches is a bit less than screen width, you'll see a truncated description for each match, and if the longest wraps the screen, you'll see the descriptions again. Minimizing the number of lines used is the reason for all of this. Some people might prefer to always see the whole description. At least this makes it obvious where you'd put a zstyle hook for that. diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c index ee39185..f5e6ba1 100644 --- a/Src/Zle/computil.c +++ b/Src/Zle/computil.c @@ -644,35 +644,43 @@ cd_get(char **params) p += str->len; memset(p, ' ', (l = (cd_state.premaxw - str->width + CM_SPACE))); p += l; - strcpy(p, cd_state.sep); - p += cd_state.slen; - /* - * copy a character at once until no more screen width - * is available. Leave 1 character at the end of screen - * as safety margin - */ remw = zterm_columns - cd_state.premaxw - cd_state.swidth - 3; - d = str->desc; - w = MB_METASTRWIDTH(d); - if (w <= remw) - strcpy(p, d); - else { - pp = p; - while (remw > 0 && *d) { - l = MB_METACHARLEN(d); - memcpy(pp, d, l); - pp[l] = '\0'; - w = MB_METASTRWIDTH(pp); - if (w > remw) { - *pp = '\0'; - break; - } + while (remw < 0 && zterm_columns) { + /* line wrapped, use remainder of the extra line */ + remw += zterm_columns; + } + if (cd_state.slen < remw) { + strcpy(p, cd_state.sep); + p += cd_state.slen; + remw -= cd_state.slen; - pp += l; - d += l; - remw -= w; + /* + * copy a character at once until no more screen + * width is available. Leave 1 character at the + * end of screen as safety margin + */ + d = str->desc; + w = MB_METASTRWIDTH(d); + if (w <= remw) + strcpy(p, d); + else { + pp = p; + while (remw > 0 && *d) { + l = MB_METACHARLEN(d); + memcpy(pp, d, l); + pp[l] = '\0'; + w = MB_METASTRWIDTH(pp); + if (w > remw) { + *pp = '\0'; + break; + } + + pp += l; + d += l; + remw -= w; + } } } -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 13:00 Bug somewhere in verbose output for completion listing Peter Stephenson 2013-10-03 13:32 ` Peter Stephenson @ 2013-10-03 14:50 ` Bart Schaefer 2013-10-03 15:09 ` Bart Schaefer 1 sibling, 1 reply; 9+ messages in thread From: Bart Schaefer @ 2013-10-03 14:50 UTC (permalink / raw) To: Zsh Hackers' List On Oct 3, 2:00pm, Peter Stephenson wrote: } Subject: Bug somewhere in verbose output for completion listing } } I expect this to be greeted with something like the cough from the back } of the room when Krusty the Clown tells a joke --- I can't reproduce } this reliably at the moment --- but something determining output width } in completion listings (I'm using the zsh/complist module) isn't being } reset properly. This looks as though it's related to this from a few months ago: http://www.zsh.org/mla/workers/2013/msg00368.html Unfortunately I don't have any new insights. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 14:50 ` Bart Schaefer @ 2013-10-03 15:09 ` Bart Schaefer 2013-10-03 16:19 ` Bart Schaefer 2013-10-03 16:20 ` Peter Stephenson 0 siblings, 2 replies; 9+ messages in thread From: Bart Schaefer @ 2013-10-03 15:09 UTC (permalink / raw) To: Zsh Hackers' List On Oct 3, 7:50am, Bart Schaefer wrote: } } http://www.zsh.org/mla/workers/2013/msg00368.html } } Unfortunately I don't have any new insights. Well, one maybe: There's a static global struct in computil.c called cd_state that has maxmlen and minmlen fields. Those are based on the exported globals maxmlen and minmlen from compcore.c, which the comment (there's a comment!) describes as "Length of the longest/shortest match". A bit of grepping does not find anywhere that cd_state.maxmlen is reset across ZLE exit/restart. I'm a bit worried that forcibly doing so may slow down or actually break _oldlist, but maybe cd_state is the place to start looking. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 15:09 ` Bart Schaefer @ 2013-10-03 16:19 ` Bart Schaefer 2013-10-03 16:20 ` Peter Stephenson 1 sibling, 0 replies; 9+ messages in thread From: Bart Schaefer @ 2013-10-03 16:19 UTC (permalink / raw) To: Zsh Hackers' List On Oct 3, 8:09am, Bart Schaefer wrote: } } Well, one maybe: There's a static global struct in computil.c called } cd_state that has maxmlen and minmlen fields. Those are based on the } exported globals maxmlen and minmlen from compcore.c, which the comment } (there's a comment!) describes as "Length of the longest/shortest match". OK, right track, wrong details. (There is no cdstate.minmlen) The problem is with cd_state.premaxw, which is the padding width. This gets set in cd_calc() but is never changed unless it gets larger. The following superfically fixes it, but I haven't checked in detail what happens in cases where the same list is being re-used (e.g. by _oldlist) for successive completions. I also haven't checked the case of multiple completions in different groups (the case from the old thread I linked earlier), or whether the LIST_PACKED option has any effect either way. diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c index f8983c3..ee39185 100644 --- a/Src/Zle/computil.c +++ b/Src/Zle/computil.c @@ -465,6 +465,7 @@ cd_init(char *nam, char *hide, char *mlen, char *sep, cd_state.showd = disp; cd_state.maxg = cd_state.groups = cd_state.descs = 0; cd_state.maxmlen = atoi(mlen); + cd_state.premaxw = 0; itmp = zterm_columns - cd_state.swidth - 4; if (cd_state.maxmlen > itmp) cd_state.maxmlen = itmp; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 15:09 ` Bart Schaefer 2013-10-03 16:19 ` Bart Schaefer @ 2013-10-03 16:20 ` Peter Stephenson 2013-10-03 16:34 ` Peter Stephenson 2013-10-04 4:01 ` Bart Schaefer 1 sibling, 2 replies; 9+ messages in thread From: Peter Stephenson @ 2013-10-03 16:20 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 03 Oct 2013 08:09:56 -0700 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Oct 3, 7:50am, Bart Schaefer wrote: > } > } http://www.zsh.org/mla/workers/2013/msg00368.html > } > } Unfortunately I don't have any new insights. > > Well, one maybe: There's a static global struct in computil.c called > cd_state that has maxmlen and minmlen fields. Those are based on the > exported globals maxmlen and minmlen from compcore.c, which the comment > (there's a comment!) describes as "Length of the longest/shortest match". > > A bit of grepping does not find anywhere that cd_state.maxmlen is reset > across ZLE exit/restart. I'm a bit worried that forcibly doing so may > slow down or actually break _oldlist, but maybe cd_state is the place > to start looking. Thanks, I think that may have been all the assistance that was needed. I discovered that before the problem took effect, completing after wtf gave (gdb) p cd_state $1 = {showd = 1, sep = 0xa0f64e8 "# ", slen = 2, swidth = 2, maxmlen = 40, sets = 0xa1183b0, pre = 10, premaxw = 10, suf = 36, maxg = 1, maxglen = 12, groups = 0, descs = 2, gprew = 0, runs = 0x0} and with the problem showing up it gave (gdb) p cd_state $2 = {showd = 1, sep = 0xa0f64e8 "# ", slen = 2, swidth = 2, maxmlen = 40, sets = 0xa1183b0, pre = 10, premaxw = 102, suf = 36, maxg = 1, maxglen = 12, groups = 0, descs = 2, gprew = 0, runs = 0x0} so premaxw has gone up. Sure enough it doesn't get reset with most of the other stuff in cd_init(), and I can't see any reason why it shouldn't be. It must start off initially at 0 so resetting it to 0 each time we initialise the state can't make anything worse (theoretically). This removes the persistent problem: you still don't get descriptions with very long completions, even though list-separator is output, which seems a dodgy combination of things, but that's obviously a different issue. diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c index f8983c3..ee39185 100644 --- a/Src/Zle/computil.c +++ b/Src/Zle/computil.c @@ -465,6 +465,7 @@ cd_init(char *nam, char *hide, char *mlen, char *sep, cd_state.showd = disp; cd_state.maxg = cd_state.groups = cd_state.descs = 0; cd_state.maxmlen = atoi(mlen); + cd_state.premaxw = 0; itmp = zterm_columns - cd_state.swidth - 4; if (cd_state.maxmlen > itmp) cd_state.maxmlen = itmp; pws ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 16:20 ` Peter Stephenson @ 2013-10-03 16:34 ` Peter Stephenson 2013-10-04 4:01 ` Bart Schaefer 1 sibling, 0 replies; 9+ messages in thread From: Peter Stephenson @ 2013-10-03 16:34 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 03 Oct 2013 17:20:07 +0100 Peter Stephenson <p.stephenson@samsung.com> wrote: > diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c > index f8983c3..ee39185 100644 > --- a/Src/Zle/computil.c > +++ b/Src/Zle/computil.c > @@ -465,6 +465,7 @@ cd_init(char *nam, char *hide, char *mlen, char *sep, > cd_state.showd = disp; > cd_state.maxg = cd_state.groups = cd_state.descs = 0; > cd_state.maxmlen = atoi(mlen); > + cd_state.premaxw = 0; > itmp = zterm_columns - cd_state.swidth - 4; > if (cd_state.maxmlen > itmp) > cd_state.maxmlen = itmp; > > pws Bart wrote: > diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c > index f8983c3..ee39185 100644 > --- a/Src/Zle/computil.c > +++ b/Src/Zle/computil.c > @@ -465,6 +465,7 @@ cd_init(char *nam, char *hide, char *mlen, char *sep, > cd_state.showd = disp; > cd_state.maxg = cd_state.groups = cd_state.descs = 0; > cd_state.maxmlen = atoi(mlen); > + cd_state.premaxw = 0; > itmp = zterm_columns - cd_state.swidth - 4; > if (cd_state.maxmlen > itmp) > cd_state.maxmlen = itmp; These look pretty similar. I'll let you commit yours. pws ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug somewhere in verbose output for completion listing 2013-10-03 16:20 ` Peter Stephenson 2013-10-03 16:34 ` Peter Stephenson @ 2013-10-04 4:01 ` Bart Schaefer 1 sibling, 0 replies; 9+ messages in thread From: Bart Schaefer @ 2013-10-04 4:01 UTC (permalink / raw) To: Zsh Hackers' List A final thought. On Oct 3, 5:20pm, Peter Stephenson wrote: } } so premaxw has gone up. Sure enough it doesn't get reset with most of the } other stuff in cd_init(), and I can't see any reason why it shouldn't } be. It must start off initially at 0 so resetting it to 0 each time } we initialise the state can't make anything worse (theoretically). We're now back to the state that Felipe Contreras complained about in the earlier thread that I referenced: torch% _foobar () { local -a commands extra commands=('one:command one' 'two:command two') _describe -t commands 'commands' commands extra=('extraone:extra command one' 'zbiggertoshowthealignissue:extra command two') _describe -t extra 'extra' extra } torch% compdef _foobar foobar torch% foobar extraone -- extra command one one -- command one two -- command two zbiggertoshowthealignissue -- extra command two torch% This happens because there are two calls to _describe for the different groups, and each call to _describe calls compdescribe -I which then resets the maximum width. (Previously it would look strange like this the first time you tried it, and then [because premaxw was NOT reset] they would all line up on the widest stance on subsequent attempts. Now at least it's consistently "mis-aligned" every time.) The behavior looks a little better if you sort the matches by group: torch% zstyle :completion:\* group-name '' torch% foobar one -- command one two -- command two extraone -- extra command one zbiggertoshowthealignissue -- extra command two To add a bit to the discussion from the previous thread, this is as it is because the entire "one -- command one" (et al.) is assembled as a single string by compdescribe. The data structure does not store pairs of (match,description) that can be realigned upon display. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-10-04 4:01 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-10-03 13:00 Bug somewhere in verbose output for completion listing Peter Stephenson 2013-10-03 13:32 ` Peter Stephenson 2013-10-03 22:54 ` Bart Schaefer 2013-10-03 14:50 ` Bart Schaefer 2013-10-03 15:09 ` Bart Schaefer 2013-10-03 16:19 ` Bart Schaefer 2013-10-03 16:20 ` Peter Stephenson 2013-10-03 16:34 ` Peter Stephenson 2013-10-04 4:01 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).