* completion: git: --fixup: problem with _describe -2V and duplicate commit subjects @ 2015-03-23 22:05 Daniel Hahler 2015-03-24 0:07 ` Daniel Hahler 0 siblings, 1 reply; 22+ messages in thread From: Daniel Hahler @ 2015-03-23 22:05 UTC (permalink / raw) To: Zsh Hackers' List -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, I've noticed that with duplicate descriptions for different commits, the new completion for --fixup (__git_recent_commits) does not behave properly, triggered by the "-2V" used with _describe. When completing "git commit --fixup=" in a repo with two commits having "init" as its subject: Without "-2V": -- commit object name -- b1fc0ca 05a98c0 -- init With "-2V": -- commit object name -- - -- init -- commit object name -- b1fc0ca 05a98c0 This does not only affect the duplicate commits, but will move all hashes to a separate "commit object name" section, leaving the descriptions without them. This was added in 236da69, where the options are passed on to next_label. Regards, Daniel. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iD8DBQFVEI4jfAK/hT/mPgARAmF/AKDwkOg7vI6dxorebP0u7ykceLuV5gCcDBOj 0H8Vrng3rwCE8mmTU6tK4x0= =5L0N -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects 2015-03-23 22:05 completion: git: --fixup: problem with _describe -2V and duplicate commit subjects Daniel Hahler @ 2015-03-24 0:07 ` Daniel Hahler 2015-03-29 5:47 ` Daniel Shahaf 0 siblings, 1 reply; 22+ messages in thread From: Daniel Hahler @ 2015-03-24 0:07 UTC (permalink / raw) To: Zsh Hackers' List -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 23.03.2015 23:05, I wrote: > I've noticed that with duplicate descriptions for different commits, > the new completion for --fixup (__git_recent_commits) does not behave > properly, triggered by the "-2V" used with _describe. > > When completing "git commit --fixup=" in a repo with two commits having > "init" as its subject: > > Without "-2V": > > -- commit object name -- > b1fc0ca 05a98c0 -- init > > With "-2V": > > -- commit object name -- > -- init > -- commit object name -- > b1fc0ca 05a98c0 > > This does not only affect the duplicate commits, but will move all > hashes to a separate "commit object name" section, leaving the > descriptions without them. > > This was added in 236da69, where the options are passed on to next_label. As a workaround I could imagine also adding the date, which is useful by itself: diff --git i/Completion/Unix/Command/_git w/Completion/Unix/Command/_git index 5524cb0..d1b3617 100644 - --- i/Completion/Unix/Command/_git +++ w/Completion/Unix/Command/_git @@ -5644,14 +5644,14 @@ __git_commit_objects () { __git_recent_commits () { local gitdir expl start declare -a descr tags heads commits - - local i j k + local i j k l # Careful: most %d will expand to the empty string. Quote properly! - - : "${(A)commits::=${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s')"}}" + : "${(A)commits::=${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s%n%cd')"}}" __git_command_successful $pipestatus || return 1 - - for i j k in "$commits[@]" ; do - - descr+=($i:$k) + for i j k l in "$commits[@]" ; do + descr+=($i:"$k ($l)") j=${${j# \(}%\)} # strip leading ' (' and trailing ')' for j in ${(s:, :)j}; do if [[ $j == 'tag: '* ]] ; then It would be nice if this could be shortened, e.g. by removing the timezone offset and removing the current year, month etc. There's a "relative date" option ("%cr"), but that might produce duplicates again. If this can be fixed, it might be a good alternative to "%cd". I've noticed btw that with `zsh -f` this is not the same, but commits are not grouped together and appear to get sorted. Therefore some zstyle might be involved here. Regards, Daniel. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iD8DBQFVEKrUfAK/hT/mPgARAhroAKCgOQca7N6owZ7WzlOfLVe5Y8D2hgCgg/Jf hLLdL0+U0tqv23HgBXeTw+o= =YFev -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects 2015-03-24 0:07 ` Daniel Hahler @ 2015-03-29 5:47 ` Daniel Shahaf 2015-03-30 18:21 ` Daniel Shahaf ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Daniel Shahaf @ 2015-03-29 5:47 UTC (permalink / raw) To: Daniel Hahler; +Cc: Zsh Hackers' List [ compdescribe question inside ] On Tue, Mar 24, 2015 at 01:07:48AM +0100, Daniel Hahler wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > Inline PGP and inline patches don't go well together (inline PGP corrupts lines with a '-' on the first column.). It would be slightly easier to process your patches if you either attached them, or used MIME PGP rather than inline PGP. Thanks. > On 23.03.2015 23:05, I wrote: > > > I've noticed that with duplicate descriptions for different commits, > > the new completion for --fixup (__git_recent_commits) does not behave > > properly, triggered by the "-2V" used with _describe. > > > > When completing "git commit --fixup=" in a repo with two commits having > > "init" as its subject: > > > > Without "-2V": > > > > -- commit object name -- > > b1fc0ca 05a98c0 -- init > > This one behaves correctly, though of course it no longer sorts completions most recent first (since -V did that). > > With "-2V": > > > > -- commit object name -- > > -- init > > -- commit object name -- > > b1fc0ca 05a98c0 > > Yes, I see that. Here's a more minimal reproducer: [[[ $ zsh -f % autoload compinit % compinit % a=(alice:foo bob:foo) % compdef '_describe -2Vx person a' f % zstyle ':completion:*:descriptions' format '%B> %U%d%u%b' % zstyle ':completion:*' group-name '' % f <TAB> > person -- foo > person alice bob % zstyle ':completion:*' completer _all_matches _complete _ignored % f <TAB> > person -- foo > person alice bob > all matches alice bob ]]] I've tracked this down a little. It has to do with the C implementation of compdescribe passing "-E1" to compadd, here: [in Src/Zle/computil.c] 737 case CRT_EXPL: 738 { 739 /* add columns as safety margin */ 740 VARARR(char, dbuf, cd_state.suf + cd_state.slen + 741 zterm_columns); 742 char buf[20], *p, *pp, *d; 743 int i = run->count, remw, w, l; 744 745 sprintf(buf, "-E%d", i); This also explains the space in front of "alice" in the "all matches" line: "compadd -E1" adds an empty match. So, in essence, compadd sees an empty match with description 'foo' and two matches, 'alice' and 'bob', with no descriptions; which causes it to print [[[ -- foo alice bob ]]] when descriptions are disabled. I'm not sure what should be done instead. One option is to disable the 'group by description' step for the 'git commit --fixup=' use case, and just let the list of matches to include two lines with the same description text. Or perhaps the "one empty match with a description and two real matches without descriptions" trick should be changed. Thoughts? > > This does not only affect the duplicate commits, but will move all > > hashes to a separate "commit object name" section, leaving the > > descriptions without them. > > > > This was added in 236da69, where the options are passed on to next_label. > > As a workaround I could imagine also adding the date, which is useful > by itself: ... > - - local i j k > + local i j k l ... > It would be nice if this could be shortened, e.g. by removing the timezone offset and > removing the current year, month etc. > I suppose you could use %ct to get the seconds-since-epoch value and strftime it yourself. However, I believe this code has to work even if the strftime builtin is not available, so it'd have to be something like "use %ci & strftime if strftime(1) is available, else fallback to %cd". > There's a "relative date" option ("%cr"), but that might produce duplicates again. > If this can be fixed, it might be a good alternative to "%cd". > Actually, I'm tempted to deduplicate it by throwing the hash into the description: diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git index 5524cb0..87ec986 100644 --- a/Completion/Unix/Command/_git +++ b/Completion/Unix/Command/_git @@ -5651,7 +5651,7 @@ __git_recent_commits () { __git_command_successful $pipestatus || return 1 for i j k in "$commits[@]" ; do - descr+=($i:$k) + descr+=("$i:[$i] $k") j=${${j# \(}%\)} # strip leading ' (' and trailing ')' for j in ${(s:, :)j}; do if [[ $j == 'tag: '* ]] ; then The hash will ensure descriptions are unique (git prints more hash digits than requested if the amount requested would be ambiguous), which works around the problem; and in the common case the [$i] prefix would be the same width in every line, making it easier to visually skip it. So I'm inclined to use the above, and fix _describe separately. We can of course add the date still, as you suggested; I just think if we add the date it should be because the date is useful, not because it uniquifies, since adding the hash is a better uniquifier. WDYT? Thanks, Daniel P.S. Sorry for the delayed reply, I was offline last week. > > I've noticed btw that with `zsh -f` this is not the same, but commits are not grouped > together and appear to get sorted. Therefore some zstyle might be involved here. > > > Regards, > Daniel. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects 2015-03-29 5:47 ` Daniel Shahaf @ 2015-03-30 18:21 ` Daniel Shahaf 2015-05-13 12:41 ` Daniel Hahler 2015-05-13 12:44 ` Daniel Hahler 2015-05-14 14:36 ` [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) Daniel Shahaf 2 siblings, 1 reply; 22+ messages in thread From: Daniel Shahaf @ 2015-03-30 18:21 UTC (permalink / raw) To: Daniel Hahler; +Cc: Zsh Hackers' List Daniel Shahaf wrote on Sun, Mar 29, 2015 at 05:47:53 +0000: > > As a workaround I could imagine also adding the date, which is useful > > by itself: ... > Actually, I'm tempted to deduplicate it by throwing the hash into the > description: Another option: add the "HEAD~15" gitrevisions(7) identifier of the commit to the description, which also uniquifies, isn't redundant, and may be easier to type. diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git index 5524cb0..00b124d 100644 --- a/Completion/Unix/Command/_git +++ b/Completion/Unix/Command/_git @@ -5645,13 +5645,27 @@ __git_recent_commits () { local gitdir expl start declare -a descr tags heads commits local i j k + integer distance_from_head # Careful: most %d will expand to the empty string. Quote properly! : "${(A)commits::=${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s')"}}" __git_command_successful $pipestatus || return 1 for i j k in "$commits[@]" ; do - descr+=($i:$k) + # Note: the after-the-colon part must be unique across the entire array; + # see workers/34768 + if (( distance_from_head == 0 )); then + descr+=($i:"[HEAD] $k") + elif (( distance_from_head == 1 )); then + descr+=($i:"[HEAD^] $k") + elif (( distance_from_head == 2 )); then + descr+=($i:"[HEAD^^] $k") + elif (( distance_from_head < 10 )); then + descr+=($i:"[HEAD~$distance_from_head] $k") + else + descr+=($i:"[HEAD~$distance_from_head] $k") + fi + (( ++distance_from_head )) j=${${j# \(}%\)} # strip leading ' (' and trailing ')' for j in ${(s:, :)j}; do if [[ $j == 'tag: '* ]] ; then I came up with this because copying the hex digits is annoying. (It's basically a captcha.) I'm half seriously considering binding '=' to a widget that inserts '=<TAB>HEAD~' if [[ $LBUFFER == *--fixup ]], to allow me to type just '--fixup=12' to get '--fixup=HEAD~12'. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects 2015-03-30 18:21 ` Daniel Shahaf @ 2015-05-13 12:41 ` Daniel Hahler 2015-05-14 14:28 ` Daniel Shahaf 0 siblings, 1 reply; 22+ messages in thread From: Daniel Hahler @ 2015-05-13 12:41 UTC (permalink / raw) To: Zsh Hackers' List -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 30.03.2015 20:21, Daniel Shahaf wrote: > Daniel Shahaf wrote on Sun, Mar 29, 2015 at 05:47:53 +0000: >>> As a workaround I could imagine also adding the date, which is useful >>> by itself: > ... >> Actually, I'm tempted to deduplicate it by throwing the hash into the >> description: > > Another option: add the "HEAD~15" gitrevisions(7) identifier of the > commit to the description, which also uniquifies, isn't redundant, and > may be easier to type. > > ... > > I came up with this because copying the hex digits is annoying. (It's > basically a captcha.) I'm half seriously considering binding '=' to > a widget that inserts '=<TAB>HEAD~' if [[ $LBUFFER == *--fixup ]], to > allow me to type just '--fixup=12' to get '--fixup=HEAD~12'. I like this idea. Have you experimented further with it? Cheers, Daniel. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iD8DBQFVU0ZsfAK/hT/mPgARApCpAKC1uGGr60H19PL3UVcG434OpuAtTACg453s wncXHgtGuvzn3mJVU4hmR4M= =n9hB -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects 2015-05-13 12:41 ` Daniel Hahler @ 2015-05-14 14:28 ` Daniel Shahaf 0 siblings, 0 replies; 22+ messages in thread From: Daniel Shahaf @ 2015-05-14 14:28 UTC (permalink / raw) To: Daniel Hahler; +Cc: Zsh Hackers' List Daniel Hahler wrote on Wed, May 13, 2015 at 14:41:16 +0200: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 30.03.2015 20:21, Daniel Shahaf wrote: > > Daniel Shahaf wrote on Sun, Mar 29, 2015 at 05:47:53 +0000: > >>> As a workaround I could imagine also adding the date, which is useful > >>> by itself: > > ... > >> Actually, I'm tempted to deduplicate it by throwing the hash into the > >> description: > > > > Another option: add the "HEAD~15" gitrevisions(7) identifier of the > > commit to the description, which also uniquifies, isn't redundant, and > > may be easier to type. > > > > ... > > > > I came up with this because copying the hex digits is annoying. (It's > > basically a captcha.) I'm half seriously considering binding '=' to > > a widget that inserts '=<TAB>HEAD~' if [[ $LBUFFER == *--fixup ]], to > > allow me to type just '--fixup=12' to get '--fixup=HEAD~12'. > > I like this idea. Have you experimented further with it? Nope, but here it is if anyone wants it: equal_for_git_fixup() { zle .self-insert if [[ $LBUFFER == *--fixup= ]] ; then zle complete-word LBUFFER+="HEAD~" fi } zle -N equal_for_git_fixup bindkey '=' equal_for_git_fixup It'd be especially useful if "HEAD~15" were offered as completions (not just as descriptions). However, we can't simply add both the hash and the HEAD~$n name of the commit to the 'descr' array, due to the bug in '_describe -2V' when a description is repeated (workers/34814). So we'd need to fix 34814 in order to add the HEAD~$n as completions. I'll post about that separately. Daniel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects 2015-03-29 5:47 ` Daniel Shahaf 2015-03-30 18:21 ` Daniel Shahaf @ 2015-05-13 12:44 ` Daniel Hahler 2015-05-14 14:36 ` [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) Daniel Shahaf 2 siblings, 0 replies; 22+ messages in thread From: Daniel Hahler @ 2015-05-13 12:44 UTC (permalink / raw) To: Zsh Hackers' List -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 29.03.2015 07:47, Daniel Shahaf wrote: > We can of course add the date still, as you suggested; I just think if > we add the date it should be because the date is useful, not because it > uniquifies, since adding the hash is a better uniquifier. I still like the date idea, especially for the recent commits. I think using "%cr" for the relative committer date, after the subject is good. It would not make it unique, but only add information. I think for uniqueness, the "HEAD~15" gitrevisions(7) identifier would be useful (instead of the hash). I will post some patch(es) later in this regard. Regards, Daniel. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iD8DBQFVU0dAfAK/hT/mPgARAuG3AKCPP/naiQiqWxXbyrzQWh2jPKr8ewCgtTIi WBInG/ASy3DqR0Jfir6Ru8o= =rCF5 -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-03-29 5:47 ` Daniel Shahaf 2015-03-30 18:21 ` Daniel Shahaf 2015-05-13 12:44 ` Daniel Hahler @ 2015-05-14 14:36 ` Daniel Shahaf 2015-05-16 22:54 ` Daniel Shahaf 2015-05-19 20:44 ` Daniel Shahaf 2 siblings, 2 replies; 22+ messages in thread From: Daniel Shahaf @ 2015-05-14 14:36 UTC (permalink / raw) To: Daniel Hahler; +Cc: Zsh Hackers' List [-- Attachment #1: Type: text/plain, Size: 1854 bytes --] Daniel Shahaf wrote on Sun, Mar 29, 2015 at 05:47:53 +0000: > [ compdescribe question inside ] This is the problem: % autoload compinit % compinit % a=(dalton:bar charlie:bar bob:foo alice:foo) % compdef '_describe -2 -V -x person a' f % compdef '_describe -2 -J -x person a' g % zstyle ':completion:*:descriptions' format '%b> %u%d%u%b' % zstyle ':completion:*' group-name '' % f <TAB> > person -- foo -- bar > person alice charlie bob dalton % g <TAB> > person alice bob -- foo charlie dalton -- bar The expected output for f is to the same output as for g, but in reverse order (the 'bar' lines should be first because they came first in the array). The executions on f and g diverge in begcmgroup() (callstack at that point: addmatches(), bin_compadd(), Completion/Base/Utility/_describe). When completing 'g', the 'alice' and 'bob' candidates get added to an existing group that already contains an empty match; when completing 'f', that group exists, but isn't matched due to flags differences (the CGF_NOSORT flag corresponds to -V), so those two matches get added to a new group, resulting in the above output. The first attached patch seems to address this. However, I encountered an odd problem with it, which I have not been able to narrow down. When I apply the first patch, Daniel Hahler's recent series, and the second attached patch, and then try to complete 'git checkout 1<TAB>',¹ I get first a screenful of hashes, and then a screenful of descriptions, and so on alternating. I'm not sure whether this indicates a bug in the first patch or an independent problem. In any case, I'm leaning towards waiting with these changes until after 5.0.8, to give them as much testing time as possible. Cheers, Daniel ¹ Those completions are generated by the function __git_commit_objects in Completion/Unix/Command/_git. [-- Attachment #2: 0001-Fix-_describe-compdescribe-problem-with-unsorted-gro.patch --] [-- Type: text/x-patch, Size: 3377 bytes --] >From 38c05e49a4beaa1dcd0398bbe6674294ab5c4a5c Mon Sep 17 00:00:00 2001 From: Daniel Shahaf <d.s@daniel.shahaf.name> Date: Thu, 14 May 2015 11:04:19 +0000 Subject: [PATCH 1/3] Fix _describe/compdescribe problem with unsorted groups (see 34814) --- Src/Zle/compcore.c | 9 +++++---- Src/Zle/computil.c | 40 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c index 000f9da..d4051bd 100644 --- a/Src/Zle/compcore.c +++ b/Src/Zle/compcore.c @@ -2996,9 +2996,9 @@ mod_export void begcmgroup(char *n, int flags) { if (n) { - Cmgroup p = amatches; - - while (p) { + /* If a group named <n> already exists, reuse it. */ + Cmgroup p; + for (p = amatches; p; p = p->next) { #ifdef ZSH_HEAP_DEBUG if (memory_validate(p->heap_id)) { HEAP_ERROR(p->heap_id); @@ -3016,9 +3016,10 @@ begcmgroup(char *n, int flags) return; } - p = p->next; } } + + /* Create a new group. */ mgroup = (Cmgroup) zhalloc(sizeof(struct cmgroup)); #ifdef ZSH_HEAP_DEBUG mgroup->heap_id = last_heap_id; diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c index a81d1dd..27938c1 100644 --- a/Src/Zle/computil.c +++ b/Src/Zle/computil.c @@ -210,6 +210,25 @@ cd_calc() } } +/* Return 1 if cd_state specifies unsorted groups, 0 otherwise. */ +static int +cd_groups_want_sorting(void) +{ + Cdset set; + char *const *i; + + for (set = cd_state.sets; set; set = set->next) + for (i = set->opts; *i; i++) { + if (!strncmp(*i, "-V", 2)) + return 0; + else if (!strncmp(*i, "-J", 2)) + return 1; + } + + /* Sorted by default */ + return 1; +} + static int cd_sort(const void *a, const void *b) { @@ -283,7 +302,8 @@ cd_prep() unmetafy(s->sortstr, &dummy); } - qsort(grps, preplines, sizeof(Cdstr), cd_sort); + if (cd_groups_want_sorting()) + qsort(grps, preplines, sizeof(Cdstr), cd_sort); for (i = preplines, strp = grps; i > 1; i--, strp++) { strp2 = strp + 1; @@ -441,7 +461,17 @@ cd_arrcat(char **a, char **b) } } -/* Initialisation. Store and calculate the string and matches and so on. */ +/* Initialisation. Store and calculate the string and matches and so on. + * + * nam: argv[0] of the builtin + * hide: ??? + * mlen: see max-matches-width style + * sep: see list-seperator style + * opts: options to (eventually) pass to compadd. + * Returned via 2nd return parameter of 'compdescribe -g'. + * args: ??? (the positional arguments to 'compdescribe') + * disp: 1 if descriptions should be shown, 0 otherwise + */ static int cd_init(char *nam, char *hide, char *mlen, char *sep, @@ -699,8 +729,12 @@ cd_get(char **params) dpys[0] = ztrdup(run->strs->str); mats[1] = dpys[1] = NULL; opts = cd_arrdup(run->strs->set->opts); + + /* Set -2V, possibly reusing the group name from an existing -J/-V + * flag. */ for (dp = opts + 1; *dp; dp++) - if (dp[0][0] == '-' && dp[0][1] == 'J') + if ((dp[0][0] == '-' && dp[0][1] == 'J') || + (dp[0][0] == '-' && dp[0][1] == 'V')) break; if (*dp) { char *s = tricat("-2V", "", dp[0] + 2); -- 1.9.1 [-- Attachment #3: 0002-git-completion-offer-HEAD-n-as-completions-in-git-re.patch --] [-- Type: text/x-patch, Size: 2152 bytes --] >From 4fdb3036dae738df6c10e959db3acf5e7575b504 Mon Sep 17 00:00:00 2001 From: Daniel Shahaf <d.s@daniel.shahaf.name> Date: Thu, 14 May 2015 11:04:51 +0000 Subject: [PATCH 2/3] git completion: offer HEAD~$n as completions in git, remove 34814 workaround --- Completion/Unix/Command/_git | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git index 9304fbb..378dc4c 100644 --- a/Completion/Unix/Command/_git +++ b/Completion/Unix/Command/_git @@ -5658,9 +5658,7 @@ __git_commit_objects () { # Abort if the argument does not match a commit hash (including empty). _guard '[[:xdigit:]](#c,40)' || return 1 - # Note: the after-the-colon part must be unique across the entire array; - # see workers/34768 - : ${(A)commits::=${(f)"$(_call_program commits git --no-pager log -1000 --all --reflog --format='%h:\[%h\]\ %s\ \(%cr\)')"}} + : ${(A)commits::=${(f)"$(_call_program commits git --no-pager log -1000 --all --reflog --format='%h:%s\ \(%cr\)')"}} __git_command_successful $pipestatus || return 1 _describe -V -t commits 'commit object name' commits @@ -5679,18 +5677,21 @@ __git_recent_commits () { __git_command_successful $pipestatus || return 1 for i j k in "$commits[@]" ; do - # Note: the after-the-colon part must be unique across the entire array; - # see workers/34768 if (( distance_from_head == 0 )); then - descr+=($i:"[HEAD] $k") + descr+=("HEAD:$k") + descr+=("$i:$k") elif (( distance_from_head == 1 )); then - descr+=($i:"[HEAD^] $k") + descr+=("HEAD^:$k") + descr+=("$i:$k") elif (( distance_from_head == 2 )); then - descr+=($i:"[HEAD^^] $k") + descr+=("HEAD^^:$k") + descr+=($i:"$k") elif (( distance_from_head < 10 )); then - descr+=($i:"[HEAD~$distance_from_head] $k") + descr+=("HEAD~$distance_from_head:$k") + descr+=($i:"$k") else - descr+=($i:"[HEAD~$distance_from_head] $k") + descr+=($i:"$k") + descr+=("HEAD~$distance_from_head:$k") fi (( ++distance_from_head )) -- 1.9.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-14 14:36 ` [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) Daniel Shahaf @ 2015-05-16 22:54 ` Daniel Shahaf 2015-05-17 4:07 ` Bart Schaefer 2015-05-19 20:44 ` Daniel Shahaf 1 sibling, 1 reply; 22+ messages in thread From: Daniel Shahaf @ 2015-05-16 22:54 UTC (permalink / raw) To: Zsh Hackers' List Daniel Shahaf wrote on Thu, May 14, 2015 at 14:36:27 +0000: > The first attached patch seems to address this. However, I encountered > an odd problem with it, which I have not been able to narrow down. When > I apply the first patch, Daniel Hahler's recent series, and the second > attached patch, and then try to complete 'git checkout 1<TAB>',¹ I get > first a screenful of hashes, and then a screenful of descriptions, and > so on alternating. I'm not sure whether this indicates a bug in the > first patch or an independent problem. To be explicit: the problem goes away if I revert the first patch, but keep the others applied. None of the other patches touch any C code or anything remotely related to the _describe internals. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-16 22:54 ` Daniel Shahaf @ 2015-05-17 4:07 ` Bart Schaefer 2015-05-17 23:40 ` Daniel Shahaf 0 siblings, 1 reply; 22+ messages in thread From: Bart Schaefer @ 2015-05-17 4:07 UTC (permalink / raw) To: Zsh Hackers' List On May 16, 10:54pm, Daniel Shahaf wrote: } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) } } Daniel Shahaf wrote on Thu, May 14, 2015 at 14:36:27 +0000: } > The first attached patch seems to address this. However, I encountered } > an odd problem with it, which I have not been able to narrow down. When } > I apply the first patch, Daniel Hahler's recent series, and the second } > attached patch, and then try to complete 'git checkout 1<TAB>',¹ I get } > first a screenful of hashes, and then a screenful of descriptions, and } > so on alternating. I'm not sure whether this indicates a bug in the } > first patch or an independent problem. } } To be explicit: the problem goes away if I revert the first patch, but } keep the others applied. You mean revert *all* of "PATCH 1/3", that is, both the compcore.c hunk (which seems to be a functional no-op) and all the hunks of computil.c? The only difference I see from eyballing the patch in 35127 is in the computil.c cd_get() hunk where before -2V would be set only on an existing -J group, but post-patch might be set on an existing -V group. I don't know why that would matter unless there is both a -J and a -V group in the same set of opts, in which case -2V is going to be set for whichever group is encountered first? Am I understanding the intent of this code correctly? In any case I can't reproduce your reported problem, at least not quite as you explain it above. Starting from commit d52bf91659522435d68f719389095001f050b6c5, I applied DH's seven patches and then your 35127, and: torch% git checkout 1<TAB> torch% git checkout 15 153a99d -- 35154: NEWS on arithmetic evaluation changes (2 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (2 days ago) 153a99d -- 35154: NEWS on arithmetic evaluation changes (2 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (2 days ago) That doesn't really look correct because it lists everything twice, but I get exactly the same thing with computil.c backed out. If I back out the rest of 35127, I get: torch% git checkout 1<TAB> torch% git checkout 15 153a99d -- [HEAD^^] 35154: NEWS on arithmetic evaluation changes (2 days a 15aa99b -- [HEAD~6] 35139: complete the new (b) parameter flag (2 days ago 153a99d -- [HEAD^^] 35154: NEWS on arithmetic evaluation changes (2 days a 15aa99b -- [HEAD~6] 35139: complete the new (b) parameter flag (2 days ago Which I guess are not strictly duplicates because the descriptions differ. If I then revert all of DH's patches (back to an empty 'git diff'): torch% git checkout 1<TAB> torch% git checkout 15 153a99d -- [153a99d] 35154: NEWS on arithmetic evaluation changes 15aa99b -- [15aa99b] 35139: complete the new (b) parameter flag The extra line for each of these is a side-effect from DH's patch 3/7 in workers/35101. Incidentally _git_recent_objects is missing a "return ret" at the end (but repairing that doesn't change the duplication). ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-17 4:07 ` Bart Schaefer @ 2015-05-17 23:40 ` Daniel Shahaf 2015-05-18 4:00 ` Bart Schaefer 0 siblings, 1 reply; 22+ messages in thread From: Daniel Shahaf @ 2015-05-17 23:40 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh Hackers' List Bart Schaefer wrote on Sat, May 16, 2015 at 21:07:35 -0700: > On May 16, 10:54pm, Daniel Shahaf wrote: > } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) > } > } Daniel Shahaf wrote on Thu, May 14, 2015 at 14:36:27 +0000: > } > The first attached patch seems to address this. However, I encountered > } > an odd problem with it, which I have not been able to narrow down. When > } > I apply the first patch, Daniel Hahler's recent series, and the second > } > attached patch, and then try to complete 'git checkout 1<TAB>',¹ I get > } > first a screenful of hashes, and then a screenful of descriptions, and > } > so on alternating. I'm not sure whether this indicates a bug in the > } > first patch or an independent problem. > } > } To be explicit: the problem goes away if I revert the first patch, but > } keep the others applied. > > You mean revert *all* of "PATCH 1/3", that is, both the compcore.c hunk > (which seems to be a functional no-op) and all the hunks of computil.c? > Yes. And yes, the compcore.c hunk is intended to be a no-op (improve readability but not affect generated machine code). > The only difference I see from eyballing the patch in 35127 is in the > computil.c cd_get() hunk where before -2V would be set only on an > existing -J group, but post-patch might be set on an existing -V group. > > I don't know why that would matter unless there is both a -J and a -V > group in the same set of opts, in which case -2V is going to be set for > whichever group is encountered first? Am I understanding the intent of > this code correctly? > It's not both a -J and a -V, but two -V's. It looks like this (when doing 'f <TAB>' using the 'f' defined at the top of 35127): compadd: '-2V-default-' '-2' '-V' 'values' '-x' '%b> %uperson%u%b' '-d' '_tmpd' '-a' '_tmpm' The '-2V-default-' is added by cd_get() line 715. The '-2 -V values' is generated by _describe (via $_type there). As you say, the first specification wins, in this case that's the "-2V-default-" group. The intent of the patch was to have compadd use the -2Vvalues group, like it uses in the 'g <TAB>' case, which works correctly, and uses: compadd: '-2' '-2V' 'values' '-x' '%b> %uperson%u%b' '-d' '_tmpd' '-a' '_tmpm' > In any case I can't reproduce your reported problem, at least not quite > as you explain it above. > > Starting from commit d52bf91659522435d68f719389095001f050b6c5, I applied > DH's seven patches and then your 35127, and: > > torch% git checkout 1<TAB> > torch% git checkout 15 > 153a99d -- 35154: NEWS on arithmetic evaluation changes (2 days ago) > 15aa99b -- 35139: complete the new (b) parameter flag (2 days ago) > 153a99d -- 35154: NEWS on arithmetic evaluation changes (2 days ago) > 15aa99b -- 35139: complete the new (b) parameter flag (2 days ago) > Okay, I repeated these exact steps, and I get: $ zsh -f % autoload compinit; compinit % git co 1<TAB> 1 153a99d -- 35154: NEWS on arithmetic evaluation changes (2 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (2 days ago) 153a99d -- 35154: NEWS on arithmetic evaluation changes (2 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (2 days ago) Note the stray '1'. I added a group-name style and then the bug reproduced. I also set tag-order to make the example output shorter, however, the bug reproduces without the tag-order setting too: [[[ % zstyle ':completion:*' group-name '' % zstyle ':completion:*' tag-order 'commits commit-objects' '*' % git checkout <TAB> %D fc8e719 4d591ef 27f99b8 d0fab2b 9c3bbbc 5c13371 4f46648 a2ed485 fcfd107 d52bf91 HEAD~10 HEAD~11 HEAD~12 HEAD~13 HEAD~14 HEAD~15 HEAD~16 HEAD~17 HEAD~18 HEAD~19 HEAD HEAD^ HEAD^^ HEAD~3 HEAD~4 HEAD~5 HEAD~6 HEAD~7 HEAD~8 HEAD~9 32a448d 153a99d 0da0a0b 59a874f e867201 15aa99b 63ffbab 55716ea 968c5ce a1c1f68 -- git completion: offer HEAD~$n as completions in git, remove 34814 workaround (11 minutes ago) -- Fix _describe/compdescribe problem with unsorted groups (see 34814) (11 minutes ago) -- completion: git: add distance_from_head to __git_recent_commits (12 minutes ago) -- completion: git: unique name for __git_recent_commits (12 minutes ago) -- completion: git: add %cr to commit objects (all and recent) (12 minutes ago) -- completion: git: __git_commit_objects: query 1000 commits (12 minutes ago) -- completion: git: add __git_commit_objects_prefer_recent (12 minutes ago) -- completion: git: use %D for %d, without the " (", ")" wrapping (12 minutes ago) -- completion: git: __git_recent_commits: remove ' ->*' from heads (12 minutes ago) -- 35155: cmdpop() could be called erroneously on error (33 hours ago) -- users/20219: fix completion for git options (2 days ago) -- 35154: NEWS on arithmetic evaluation changes (2 days ago) -- 35153: nested math substitution (2 days ago) -- 35151: improved check for parameter q and b flags (2 days ago) -- 35131: allow "[]" to match empty character set. (2 days ago) -- 35139: complete the new (b) parameter flag (2 days ago) -- Øystein Walle: 34841 (tweaked): allow grouping of thousands in printf format string (2 days ago) -- unposted: include .distfiles for new directory (2 days ago) -- 35062: __git_setup_revision_options includes __git_setup_diff_options (3 days ago) -- 35061: add __git_setup_diff_stage_options and use it with _git-diff-files and _git-diff explicitly (3 days ago) ... [snip: everything above this point, except for the %D, was repeated] 1 4 dot-zsh-3.1.5-pws-17 master zsh-4.0-patches 2 \#CVSPS.NO.BRANCH dot-zsh-3.1.5-pws-19 zsh zsh-4.2-patches 3 dot-zsh-3.1.5-pws-14 interrupt_abort zsh-3.1.5-pws-16-patches ]]] The literal %D at the start is an artifact of Daniel Hahler's 2/7 patch. (My git doesn't support the %D expando, so git outputs "%D" verbatim and _git interprets that as a head name and offers it as a completion.) > That doesn't really look correct because it lists everything twice, but > I get exactly the same thing with computil.c backed out. If I back out > the rest of 35127, I get: > > torch% git checkout 1<TAB> > torch% git checkout 15 > 153a99d -- [HEAD^^] 35154: NEWS on arithmetic evaluation changes (2 days a > 15aa99b -- [HEAD~6] 35139: complete the new (b) parameter flag (2 days ago > 153a99d -- [HEAD^^] 35154: NEWS on arithmetic evaluation changes (2 days a > 15aa99b -- [HEAD~6] 35139: complete the new (b) parameter flag (2 days ago > I get: % git co 1<TAB> 1 153a99d -- [HEAD~9] 35154: NEWS on arithmetic evaluation changes (2 days ago) 15aa99b -- [HEAD~13] 35139: complete the new (b) parameter flag (2 days ago) 153a99d -- [HEAD~9] 35154: NEWS on arithmetic evaluation changes (2 days ago) 15aa99b -- [HEAD~13] 35139: complete the new (b) parameter flag (2 days ago) There are two significant differences from your example: the stray '1' output and the fact that '1<TAB>' does not become '15' for me. The difference in bracketed parts is insignificant (I used git-am to actually apply Daniel Hahler's 7 patches as separate commits on top of HEAD, whereas you apparently applied them as local mods on top of HEAD). > Which I guess are not strictly duplicates because the descriptions differ. > The descriptions you pasted do not differ: both 153a99d entries have the same description as each other, as do both 15aa99b entries. > If I then revert all of DH's patches (back to an empty 'git diff'): > > torch% git checkout 1<TAB> > torch% git checkout 15 > 153a99d -- [153a99d] 35154: NEWS on arithmetic evaluation changes > 15aa99b -- [15aa99b] 35139: complete the new (b) parameter flag > > The extra line for each of these is a side-effect from DH's patch 3/7 in > workers/35101. > > Incidentally _git_recent_objects is missing a "return ret" at the end (but > repairing that doesn't change the duplication). > Thanks very much for looking into this! I know "apply nine patches" isn't the world's friendliest reproduction recipe, but I had nothing simpler and my head was getting flat from being hit against a wall for too long... Daniel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-17 23:40 ` Daniel Shahaf @ 2015-05-18 4:00 ` Bart Schaefer 2015-05-19 1:36 ` Daniel Shahaf 0 siblings, 1 reply; 22+ messages in thread From: Bart Schaefer @ 2015-05-18 4:00 UTC (permalink / raw) To: Zsh Hackers' List On May 17, 11:40pm, Daniel Shahaf wrote: } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) } } Bart Schaefer wrote on Sat, May 16, 2015 at 21:07:35 -0700: } > In any case I can't reproduce your reported problem, at least not quite } > as you explain it above. } > } > Starting from commit d52bf91659522435d68f719389095001f050b6c5, I applied } > DH's seven patches and then your 35127 OK, now starting from commit 34a1489f436d95bc2404f8e371130a469cbccebe, I apply 35127 and: torch% zstyle -L zstyle ':completion:*:messages' format %S%d%s zstyle ':completion:*:warnings' format 'No matches for %U%d%u' zstyle ':completion:*' format '%SCompleting %U%d%u%s' zstyle ':completion:*' group-name '' zstyle ':completion:*' ignore-parents parent pwd .. zstyle ':completion::*' insert-tab 'pending=1' zstyle ':completion:*' menu 'yes=long' 'select=long-list' zstyle ':completion:*' verbose true torch% git checkout 1<TAB> Completing recent commit object name 1d5b225 -- 35100: __git_recent_commits: massage ' ->*' from heads (3 hours 153a99d -- 35154: NEWS on arithmetic evaluation changes (3 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (3 days ago) 1d5b225 -- 35100: __git_recent_commits: massage ' ->*' from heads (3 hours 153a99d -- 35154: NEWS on arithmetic evaluation changes (3 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (3 days ago) The duplication remains annoying -- again, this is coming from commit 454f079852deb0c704870c7d5a462485f1e65bbf (workers/35101) -- but I still can't reproduce what you're seeing. I even whittled down to: torch% zstyle -L zstyle ':completion:*' group-name '' zstyle ':completion:*' tag-order 'commits commit-objects' '*' torch% git checkout 1<TAB> 1d5b225 -- 35100: __git_recent_commits: massage ' ->*' from heads (3 hours 153a99d -- 35154: NEWS on arithmetic evaluation changes (3 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (3 days ago) 1d5b225 -- 35100: __git_recent_commits: massage ' ->*' from heads (3 hours 153a99d -- 35154: NEWS on arithmetic evaluation changes (3 days ago) 15aa99b -- 35139: complete the new (b) parameter flag (3 days ago) Is there something else being found in your git tree(s) that I don't have? Then you have this: } 1 4 dot-zsh-3.1.5-pws-17 master zsh-4.0-patches } 2 \#CVSPS.NO.BRANCH dot-zsh-3.1.5-pws-19 zsh zsh-4.2-patches } 3 dot-zsh-3.1.5-pws-14 interrupt_abort zsh-3.1.5-pws-16-patches Something is generating 1 2 3 4 as possible completions. Do you perhaps have a git alias for a command that's being invoked by _git, so that the output is not as _git expects? (The "stray" 1 in your output is the 1 on the command line matching exactly the 1 in this listing.) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-18 4:00 ` Bart Schaefer @ 2015-05-19 1:36 ` Daniel Shahaf 2015-05-19 4:37 ` Bart Schaefer 0 siblings, 1 reply; 22+ messages in thread From: Daniel Shahaf @ 2015-05-19 1:36 UTC (permalink / raw) To: Zsh Hackers' List Bart Schaefer wrote on Sun, May 17, 2015 at 21:00:27 -0700: > On May 17, 11:40pm, Daniel Shahaf wrote: > } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) > } > } Bart Schaefer wrote on Sat, May 16, 2015 at 21:07:35 -0700: > } > In any case I can't reproduce your reported problem, at least not quite > } > as you explain it above. > } > > } > Starting from commit d52bf91659522435d68f719389095001f050b6c5, I applied > } > DH's seven patches and then your 35127 > > Then you have this: > > } 1 4 dot-zsh-3.1.5-pws-17 master zsh-4.0-patches > } 2 \#CVSPS.NO.BRANCH dot-zsh-3.1.5-pws-19 zsh zsh-4.2-patches > } 3 dot-zsh-3.1.5-pws-14 interrupt_abort zsh-3.1.5-pws-16-patches > > Something is generating 1 2 3 4 as possible completions. Do you perhaps > have a git alias for a command that's being invoked by _git, so that the > output is not as _git expects? (The "stray" 1 in your output is the 1 on > the command line matching exactly the 1 in this listing.) The "1 2 3 4" come from these remote branches: % git for-each-ref | grep pr/ e1a7e686156e0e450164151dd1d3be192574bed3 commit refs/remotes/github/pr/1 a1cf0253d8437aac4858e65b0a07c97243f15ec6 commit refs/remotes/github/pr/2 14e5aa4b0cf69134b36dfde72c8dec19d99bf1ff commit refs/remotes/github/pr/3 60e0f7163b9a06d8f7acd612db914ab335c65396 commit refs/remotes/github/pr/4 % git config -l | grep remote.github remote.github.url=https://github.com/zsh-users/zsh remote.github.fetch=+refs/heads/*:refs/remotes/github/* remote.github.fetch=+refs/pull/*/head:refs/remotes/github/pr/* remote.github.skipfetchall=true > OK, now starting from commit 34a1489f436d95bc2404f8e371130a469cbccebe, I > apply 35127 and: > > torch% zstyle -L > zstyle ':completion:*:messages' format %S%d%s > zstyle ':completion:*:warnings' format 'No matches for %U%d%u' > zstyle ':completion:*' format '%SCompleting %U%d%u%s' > zstyle ':completion:*' group-name '' > zstyle ':completion:*' ignore-parents parent pwd .. > zstyle ':completion::*' insert-tab 'pending=1' > zstyle ':completion:*' menu 'yes=long' 'select=long-list' > zstyle ':completion:*' verbose true > torch% git checkout 1<TAB> > Completing recent commit object name > 1d5b225 -- 35100: __git_recent_commits: massage ' ->*' from heads (3 hours > 153a99d -- 35154: NEWS on arithmetic evaluation changes (3 days ago) > 15aa99b -- 35139: complete the new (b) parameter flag (3 days ago) > 1d5b225 -- 35100: __git_recent_commits: massage ' ->*' from heads (3 hours > 153a99d -- 35154: NEWS on arithmetic evaluation changes (3 days ago) > 15aa99b -- 35139: complete the new (b) parameter flag (3 days ago) I get the same, modulo the additional remote branch. > Is there something else being found in your git tree(s) that I don't have? You tried to complete 'git checkout 1<TAB>'. The bug reproduces when I do 'git checkout <TAB>', i.e., space-tab, not space-digitone-tab. I can still reproduce the bug in the above setting (34a1489 plus 35127 plus either set of zstyles), even after I remove my ~/.gitconfig and cd to a fresh clone of git://git.code.sf.net/p/zsh/code. I haven't noticed this before, but after pressing space and tab I get a "zsh: do you wish to see all 140 possibilities (124 lines)?" prompt that I answer 'y' to. The figure 140 is because there are 100 completions and 40 descriptions; that is, the descriptions are counted towards the total number of offered completions. Why would descriptions be counted towards the total number of completions? Cheers, Daniel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-19 1:36 ` Daniel Shahaf @ 2015-05-19 4:37 ` Bart Schaefer 2015-05-19 20:41 ` Daniel Shahaf 0 siblings, 1 reply; 22+ messages in thread From: Bart Schaefer @ 2015-05-19 4:37 UTC (permalink / raw) To: Zsh Hackers' List On May 19, 1:36am, Daniel Shahaf wrote: } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) } } You tried to complete 'git checkout 1<TAB>'. The bug reproduces when } I do 'git checkout <TAB>', i.e., space-tab, not space-digitone-tab. OK; I was just trying to repeat what you said in 35127: > I apply the first patch, Daniel Hahler's recent series, and the second > attached patch, and then try to complete 'git checkout 1<TAB>',¹ I get > first a screenful of hashes, and then a screenful of descriptions, and I now realize that you don't mean "a screenful of ######" but rather a screenful of git checksums. I've been looking all this time for some sort of horrible memory scrambling that was filling your screen with garbage. Sigh. So ... 35127 deletes/disregards this comment: - # Note: the after-the-colon part must be unique across the entire array; - # see workers/34768 It then proceeds to add two completions for each description. The problem described in 34768 kicks in, and you get the results you observed. If I modify the _git hunk in 35127 to force one $k part of each pair of _descr+=(...) assignments to differ, everything works as desired, except that there are still another duplicate set of everything from 35101. } I haven't noticed this before, but after pressing space and tab I get } a "zsh: do you wish to see all 140 possibilities (124 lines)?" prompt } that I answer 'y' to. The figure 140 is because there are 100 } completions and 40 descriptions; that is, the descriptions are counted } towards the total number of offered completions. Why would descriptions } be counted towards the total number of completions? Side-effect of the problem discussed in 34768, and a clue if we'd noted it sooner. I think it's up to you to figure out how to make 34671 work with 35127, as they are both your patches. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-19 4:37 ` Bart Schaefer @ 2015-05-19 20:41 ` Daniel Shahaf 2015-05-19 22:03 ` Bart Schaefer 0 siblings, 1 reply; 22+ messages in thread From: Daniel Shahaf @ 2015-05-19 20:41 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh Hackers' List Bart Schaefer wrote on Mon, May 18, 2015 at 21:37:14 -0700: > On May 19, 1:36am, Daniel Shahaf wrote: > } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) > } > } You tried to complete 'git checkout 1<TAB>'. The bug reproduces when > } I do 'git checkout <TAB>', i.e., space-tab, not space-digitone-tab. > > OK; I was just trying to repeat what you said in 35127: > > I apply the first patch, Daniel Hahler's recent series, and the second > > attached patch, and then try to complete 'git checkout 1<TAB>',¹ I get > > first a screenful of hashes, and then a screenful of descriptions, and > > I now realize that you don't mean "a screenful of ######" but rather a > screenful of git checksums. I've been looking all this time for some > sort of horrible memory scrambling that was filling your screen with > garbage. Sigh. Sorry about the miscommunication. I see my original phrasing was ambiguous. I don't understand why 35169 didn't resolve the ambiguity (that post includes a verbatim copy of the garbled output). > So ... 35127 deletes/disregards this comment: > > - # Note: the after-the-colon part must be unique across the entire array; > - # see workers/34768 > > It then proceeds to add two completions for each description. The problem > described in 34768 kicks in, and you get the results you observed. The second patch in 35127 deliberately triggers the condition the being-removed comment states is buggy, to prove that the first patch in 35127 fixes that particular bug. The issue is now resolved; I'll write a separate email with details. (Spoiler: 35127 is correct when applied on top of 35216.) Again, sorry for the miscommunication, and thanks for the help. Daniel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-19 20:41 ` Daniel Shahaf @ 2015-05-19 22:03 ` Bart Schaefer 0 siblings, 0 replies; 22+ messages in thread From: Bart Schaefer @ 2015-05-19 22:03 UTC (permalink / raw) To: Zsh Hackers' List On May 19, 8:41pm, Daniel Shahaf wrote: } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) } } Bart Schaefer wrote on Mon, May 18, 2015 at 21:37:14 -0700: } > I now realize that you don't mean "a screenful of ######" but rather a } > screenful of git checksums. } } Sorry about the miscommunication. I see my original phrasing was } ambiguous. I don't understand why 35169 didn't resolve the ambiguity } (that post includes a verbatim copy of the garbled output). I thought they were two different things, one that occurred with nothing after "checkout" and one that occurred when completing a partial string after "checkout". } The second patch in 35127 deliberately triggers the condition the } being-removed comment states is buggy, to prove that the first patch in } 35127 fixes that particular bug. OK, so far I'm taking your word for it. :-) -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-14 14:36 ` [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) Daniel Shahaf 2015-05-16 22:54 ` Daniel Shahaf @ 2015-05-19 20:44 ` Daniel Shahaf 2015-05-19 21:01 ` Daniel Shahaf 2015-05-19 22:23 ` Bart Schaefer 1 sibling, 2 replies; 22+ messages in thread From: Daniel Shahaf @ 2015-05-19 20:44 UTC (permalink / raw) To: Zsh Hackers' List; +Cc: Daniel Hahler [-- Attachment #1: Type: text/plain, Size: 1735 bytes --] Daniel Shahaf wrote on Thu, May 14, 2015 at 14:36:27 +0000: > The first attached patch seems to address this. However, > I encountered an odd problem with it, which I have not been able to > narrow down. [... 'git checkout 1<TAB>' ...] I'm not sure whether > this indicates a bug in the first patch or an independent problem. The 'git checkout <TAB>' issue is an independent problem. Daniel Hahler has determined that the garbled output is due to _git's calling __git_commit_objects twice (cf 35216). The attached minimal example demonstrates the problem. So, in summary: 1. The first patch in 35127 fixes the problem introduced by 34671, reported in 34768, and analysed in 34814. The minimal example at the top of 35127 produces garbled output without 35127 and correct output with 35127. 2. The second patch in 35127 triggered garbled output in 'git checkout <TAB>' because of a preexisting problem in _git, which 35216 fixes. An example of the garbled output is in 35169, surrounded by triple square brackets. 3. I propose the following documentation patch: diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo index a081ea3..5043e7b 100644 --- a/Doc/Zsh/compsys.yo +++ b/Doc/Zsh/compsys.yo @@ -4197,6 +4197,9 @@ description will appear together in the list. tt(_describe) uses the tt(_all_labels) function to generate the matches, so it does not need to appear inside a loop over tag labels. + +tt(_describe) should not be called more than once in a single call to +a completion function. ) findex(_description) item(tt(_description) [ tt(-x) ] [ tt(-12VJ) ] var(tag) var(name) var(descr) [ var(spec) ... ])( Unless objections, I'll commit 35127#1 (the first of the two patches attached to that email). Daniel [-- Attachment #2: ftwice.txt --] [-- Type: text/plain, Size: 242 bytes --] a=(dalton:bar charlie:bar bob:foo alice:foo) _ftwice() { _describe -2 -V -x person a; _describe -2 -V -x person a }; compdef _ftwice ftwice % ftwice <TAB> > person charlie alice dalton bob -- bar -- foo charlie alice dalton bob -- bar -- foo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-19 20:44 ` Daniel Shahaf @ 2015-05-19 21:01 ` Daniel Shahaf 2015-05-19 22:23 ` Bart Schaefer 1 sibling, 0 replies; 22+ messages in thread From: Daniel Shahaf @ 2015-05-19 21:01 UTC (permalink / raw) To: Zsh Hackers' List; +Cc: Daniel Hahler Daniel Shahaf wrote on Tue, May 19, 2015 at 20:44:46 +0000: > Daniel Shahaf wrote on Thu, May 14, 2015 at 14:36:27 +0000: > > The first attached patch seems to address this. However, > > I encountered an odd problem with it, which I have not been able to > > narrow down. [... 'git checkout 1<TAB>' ...] I'm not sure whether > > this indicates a bug in the first patch or an independent problem. > > The 'git checkout <TAB>' issue is an independent problem. > > Daniel Hahler has determined that the garbled output is due to _git's > calling __git_commit_objects twice (cf 35216). The attached minimal > example demonstrates the problem. > > So, in summary: > > 1. The first patch in 35127 fixes the problem introduced by 34671, > reported in 34768, and analysed in 34814. The minimal example at the > top of 35127 produces garbled output without 35127 and correct output > with 35127. > > 2. The second patch in 35127 triggered garbled output in 'git checkout > <TAB>' because of a preexisting problem in _git, which 35216 fixes. > An example of the garbled output is in 35169, surrounded by triple > square brackets. > With both 35127 and 35169 applied, I get the :descriptions message twice: > recent commit object name > recent commit object name cf83173 HEAD - debug (12 minutes ago) ... The duplicate goes away if I revert ed3e5f521d1243981831d7e084225b03e205ae38 (35209), i.e., add back the -2. I verified that __git_recent_commits is called only once, so 35169 is not at fault here. I'm not sure what the actual problem is: whether 35209 should be reverted (the -2 added back), or 35127#1 should be smarter about how compdescribe calls compadd (cf empty matches discussed in 34814). I'll investigate later. Cheers, Daniel > 3. I propose the following documentation patch: > > diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo > index a081ea3..5043e7b 100644 > --- a/Doc/Zsh/compsys.yo > +++ b/Doc/Zsh/compsys.yo > @@ -4197,6 +4197,9 @@ description will appear together in the list. > > tt(_describe) uses the tt(_all_labels) function to generate the matches, so > it does not need to appear inside a loop over tag labels. > + > +tt(_describe) should not be called more than once in a single call to > +a completion function. > ) > findex(_description) > item(tt(_description) [ tt(-x) ] [ tt(-12VJ) ] var(tag) var(name) var(descr) [ var(spec) ... ])( > > Unless objections, I'll commit 35127#1 (the first of the two patches > attached to that email). > > Daniel > a=(dalton:bar charlie:bar bob:foo alice:foo) > _ftwice() { _describe -2 -V -x person a; _describe -2 -V -x person a }; compdef _ftwice ftwice > % ftwice <TAB> > > person > charlie > alice > dalton > bob > -- bar > -- foo > charlie > alice > dalton > bob > -- bar > -- foo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-19 20:44 ` Daniel Shahaf 2015-05-19 21:01 ` Daniel Shahaf @ 2015-05-19 22:23 ` Bart Schaefer 2015-05-20 23:51 ` Daniel Shahaf 1 sibling, 1 reply; 22+ messages in thread From: Bart Schaefer @ 2015-05-19 22:23 UTC (permalink / raw) To: Zsh Hackers' List On May 19, 8:44pm, Daniel Shahaf wrote: } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) } } 3. I propose the following documentation patch: } } +tt(_describe) should not be called more than once in a single call to } +a completion function. Is it really any time _describe is called twice, or is it just that _describe should not be called more than once with the same (or an overlapping?) set of descriptions? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-19 22:23 ` Bart Schaefer @ 2015-05-20 23:51 ` Daniel Shahaf 2015-05-22 5:02 ` Bart Schaefer 2015-05-23 10:03 ` Daniel Shahaf 0 siblings, 2 replies; 22+ messages in thread From: Daniel Shahaf @ 2015-05-20 23:51 UTC (permalink / raw) To: Zsh Hackers' List Bart Schaefer wrote on Tue, May 19, 2015 at 15:23:00 -0700: > On May 19, 8:44pm, Daniel Shahaf wrote: > } Subject: Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) > } > } 3. I propose the following documentation patch: > } > } +tt(_describe) should not be called more than once in a single call to > } +a completion function. > > Is it really any time _describe is called twice, or is it just that > _describe should not be called more than once with the same (or an > overlapping?) set of descriptions? If you call _describe more than once, with the same tag parameter in both calls (or with no -t parameter in both calls), and each of the two calls (separately) has a pair of items with equal description, then the display will be garbled. This happens even if no flags (other than possibly -t) are passed. I'm not sure whether that's a complete characterization, i.e., whether there are other circumstances that also trigger the problem. I think the trick might be to get cd_get() to pass -E to compadd (inserting an empty match) twice during one completion operation. While looking into this, I've also found a 100% CPU consumption bug; to reproduce (in master): autoload compinit compinit _f() { d=(d1); a=(); compadd -XX -JJ -E1 -d d -a a; compadd -XX -JJ -E1 -d d -a a }; compdef _f f f <TAB><TAB><TAB> Backtrace: 0x00007ffff60b7a5d in do_menucmp (lst=4) at compresult.c:1234 1234 } while (!(minfo.group)->mcount); (gdb) bt #0 0x00007ffff60b7a5d in do_menucmp (lst=4) at compresult.c:1234 #1 0x00007ffff60a2aaf in before_complete (dummy=0x7ffff6516c30, lst=0x7fffffffdd5c) at compcore.c:478 #2 0x000000000046870f in runhookdef (h=0x7ffff6516c30, d=0x7fffffffdd5c) at module.c:996 #3 0x00007ffff62f7084 in docomplete (lst=4) at zle_tricky.c:622 #4 0x00007ffff62f66b7 in expandorcomplete (args=0x7ffff6517188) at zle_tricky.c:315 It's spinning in the outer of the two do-while loops in there, with minfo.cur being a dummy match. Cheers, Daniel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-20 23:51 ` Daniel Shahaf @ 2015-05-22 5:02 ` Bart Schaefer 2015-05-23 10:03 ` Daniel Shahaf 1 sibling, 0 replies; 22+ messages in thread From: Bart Schaefer @ 2015-05-22 5:02 UTC (permalink / raw) To: Zsh Hackers' List On May 20, 11:51pm, Daniel Shahaf wrote: } } While looking into this, I've also found a 100% CPU consumption bug; to } reproduce (in master): } } autoload compinit } compinit } _f() { d=(d1); a=(); compadd -XX -JJ -E1 -d d -a a; compadd -XX -JJ -E1 -d d -a a }; compdef _f f } f <TAB><TAB><TAB> } } It's spinning in the outer of the two do-while loops in there, with } minfo.cur being a dummy match. It reaches the state where it has to try amatches, but the "default" group to which amatches points contains only a CMF_DUMMY, so it won't display it; thus it tries to go to the next match, but there are no more, so back to amatches we go again, and loop. So the obvious thing would seem to be to stop the loop if we arrive at the "examine amatches" state twice, which is easy enough to do; but then on a subsequent (fourth, if repeating the example above) attempt, *++(minfo.cur) runs off the end of memory and the shell crashes on the subsequent access through (*minfo.cur). Thus the less-obvious thing to do is, upon discovering the twice-amatches case, force everything back to the state it was in the second-to-last time do_menucmp() was called, so we just loop displaying the menu in a stable way. Trouble is, I'm not sure where that state can be found. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) 2015-05-20 23:51 ` Daniel Shahaf 2015-05-22 5:02 ` Bart Schaefer @ 2015-05-23 10:03 ` Daniel Shahaf 1 sibling, 0 replies; 22+ messages in thread From: Daniel Shahaf @ 2015-05-23 10:03 UTC (permalink / raw) To: Zsh Hackers' List Daniel Shahaf wrote on Wed, May 20, 2015 at 23:51:58 +0000: > While looking into this, I've also found a 100% CPU consumption bug; to > reproduce (in master): So, another summary... - 35127#1 (fix for _describe -2V with duplicate descriptions): no reason to hold it back any longer; I'll push it. - 35227 (duplicate :descriptions message): cosmetic issue. The example given in that email requires 35127#2, which I'm not going to push yet, and no other example is known, so this issue is not blocking. - 35246#tail (infinite loop): pre-existing problem, not related to my changes (it also happens in 5.0.7, which precedes my changes). No known instances "in the wild". I'm not currently looking into it. - 35246#head (_describe output garbled): likewise, a pre-existing problem with no known instances. Enclosed a patch to record its existence. - Empty matches, probably added by cd_get(), are definitely related to the third issue and may be related to the second and fourth as well. Is there better way to track all these issues than X-Seq numbers? --- diff --git a/Completion/Base/Utility/_describe b/Completion/Base/Utility/_describe index ab72005..76ab1d9 100644 --- a/Completion/Base/Utility/_describe +++ b/Completion/Base/Utility/_describe @@ -1,5 +1,12 @@ #autoload +# ### Note: Calling this function twice during one completion operation, such +# ### that in each call there exists a pair of items having the same description +# ### as each other, and the two calls specify the same $_type, currently leads +# ### to garbled output; see workers/35229 (May 2015) and its thread (which also +# ### discusses at least two other issues, that may or may not be related to +# ### this one). + # This can be used to add options or values with descriptions as matches. local _opt _expl _tmpm _tmpd _mlen _noprefix ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-05-23 10:03 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-23 22:05 completion: git: --fixup: problem with _describe -2V and duplicate commit subjects Daniel Hahler 2015-03-24 0:07 ` Daniel Hahler 2015-03-29 5:47 ` Daniel Shahaf 2015-03-30 18:21 ` Daniel Shahaf 2015-05-13 12:41 ` Daniel Hahler 2015-05-14 14:28 ` Daniel Shahaf 2015-05-13 12:44 ` Daniel Hahler 2015-05-14 14:36 ` [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects) Daniel Shahaf 2015-05-16 22:54 ` Daniel Shahaf 2015-05-17 4:07 ` Bart Schaefer 2015-05-17 23:40 ` Daniel Shahaf 2015-05-18 4:00 ` Bart Schaefer 2015-05-19 1:36 ` Daniel Shahaf 2015-05-19 4:37 ` Bart Schaefer 2015-05-19 20:41 ` Daniel Shahaf 2015-05-19 22:03 ` Bart Schaefer 2015-05-19 20:44 ` Daniel Shahaf 2015-05-19 21:01 ` Daniel Shahaf 2015-05-19 22:23 ` Bart Schaefer 2015-05-20 23:51 ` Daniel Shahaf 2015-05-22 5:02 ` Bart Schaefer 2015-05-23 10:03 ` Daniel Shahaf
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).