* Patch 1/3: Fix prefix insertion logic @ 2023-04-29 18:07 Marlon Richert 2023-04-29 18:08 ` Patch 2/3: Make dynamic dir completion easier to implement Marlon Richert 2023-04-29 18:58 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras 0 siblings, 2 replies; 27+ messages in thread From: Marlon Richert @ 2023-04-29 18:07 UTC (permalink / raw) To: Zsh hackers list [-- Attachment #1.1: Type: text/plain, Size: 1 bytes --] [-- Attachment #1.2: Type: text/html, Size: 96 bytes --] [-- Attachment #2: 0001-51641-Fix-_prefix-insertion-logic.txt --] [-- Type: text/plain, Size: 2609 bytes --] From 6a4f094ebd11e10a03d8c10016a0f6de392022ba Mon Sep 17 00:00:00 2001 From: Marlon Richert <marlon.richert@hibox.tv> Date: Sat, 29 Apr 2023 19:25:37 +0300 Subject: [PATCH 1/3] 51641: Fix _prefix insertion logic --- Completion/Base/Completer/_prefix | 9 +-------- Test/Y01completion.ztst | 18 ++++++++++++++++++ Test/comptest | 2 +- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/Completion/Base/Completer/_prefix b/Completion/Base/Completer/_prefix index 74be5f47d..01739166e 100644 --- a/Completion/Base/Completer/_prefix +++ b/Completion/Base/Completer/_prefix @@ -49,14 +49,7 @@ for tmp in "$comp[@]"; do fi if [[ "$tmp" != _prefix ]] && "$tmp"; then - [[ compstate[nmatches] -gt 1 ]] && return 0 - compadd -U -i "$IPREFIX" -I "$ISUFFIX" - "${compstate[unambiguous]%$suf}x" - compstate[list]= - if [[ -n $compstate[unambiguous] ]]; then - compstate[insert]=unambiguous - else - compstate[insert]=0 - fi + compstate[to_end]='' return 0 fi (( _matcher_num++ )) diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst index f976f9f91..51e246307 100644 --- a/Test/Y01completion.ztst +++ b/Test/Y01completion.ztst @@ -63,6 +63,24 @@ >NO:{file1} >NO:{file2} + comptesteval 'tst-insert() { compstate[insert]=1; compstate[list]= }' + comptesteval 'comppostfuncs=( tst-insert )' + comptest $': dir1\ebf\t' +0:_prefix with compstate[insert]=1 does not move to end +>line: {: file1}{dir1} + + comptesteval 'unfunction tst-insert' + comptesteval 'comppostfuncs=()' + comptest $': dir1\ebf\t\t\t\t' +0:_prefix inserts unambiguous and does not move to end +>line: {: file}{dir1} +>line: {: file}{dir1} +>DESCRIPTION:{file} +>FI:{file1} +>FI:{file2} +>line: {: file1}{dir1} +>line: {: file2}{dir1} + comptesteval $'zstyle -d \'*\' glob' comptesteval '_users () { compadd user1 user2 }' comptest $': ~\t\t\t\t\t' diff --git a/Test/comptest b/Test/comptest index 79c69979a..a57f4bcc4 100644 --- a/Test/comptest +++ b/Test/comptest @@ -40,7 +40,7 @@ KEYTIMEOUT=1 setopt zle autoload -U compinit compinit -u -zstyle ":completion:*" completer _expand _complete _ignored +zstyle ":completion:*" completer _expand _complete _prefix _ignored zstyle ":completion:*:default" list-colors "no=<NO>" "fi=<FI>" "di=<DI>" "ln=<LN>" "pi=<PI>" "so=<SO>" "bd=<BD>" "cd=<CD>" "ex=<EX>" "mi=<MI>" "tc=<TC>" "sp=<SP>" "lc=<LC>" "ec=<EC>\n" "rc=<RC>" zstyle ":completion:*" group-name "" zstyle ":completion:*:messages" format "<MESSAGE>%d</MESSAGE> -- 2.39.2 (Apple Git-143) ^ permalink raw reply [flat|nested] 27+ messages in thread
* Patch 2/3: Make dynamic dir completion easier to implement 2023-04-29 18:07 Patch 1/3: Fix prefix insertion logic Marlon Richert @ 2023-04-29 18:08 ` Marlon Richert 2023-04-29 18:09 ` Patch 3/3: Fix subscript completion bugs inside ~[...] Marlon Richert 2023-04-29 18:58 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras 1 sibling, 1 reply; 27+ messages in thread From: Marlon Richert @ 2023-04-29 18:08 UTC (permalink / raw) To: Zsh hackers list [-- Attachment #1.1: Type: text/plain, Size: 1 bytes --] [-- Attachment #1.2: Type: text/html, Size: 22 bytes --] [-- Attachment #2: 0002-Make-dynamic-dir-completion-easier-to-implement.txt --] [-- Type: text/plain, Size: 4046 bytes --] From 8e158d247cba285a5df1441ee11d39af8a75eb19 Mon Sep 17 00:00:00 2001 From: Marlon Richert <marlon.richert@hibox.tv> Date: Sat, 29 Apr 2023 20:59:03 +0300 Subject: [PATCH 2/3] Make dynamic dir completion easier to implement --- .../Zsh/Context/_dynamic_directory_name | 30 +++++++++---- Doc/Zsh/expn.yo | 44 +++++++------------ Test/Y01completion.ztst | 11 +++++ 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/Completion/Zsh/Context/_dynamic_directory_name b/Completion/Zsh/Context/_dynamic_directory_name index f449c3b12..e173dcb7f 100644 --- a/Completion/Zsh/Context/_dynamic_directory_name +++ b/Completion/Zsh/Context/_dynamic_directory_name @@ -1,15 +1,29 @@ #autoload +local -a dirfuncs=( + ${(k)functions[zsh_directory_name]} + $zsh_directory_name_functions +) +local descr='dynamically named directory' -local func -integer ret=1 +if (( $#dirfuncs )); then + local -a expl=() + local -i ret=1 + local func= + local tag=dynamically-named-directories + [[ -z $ISUFFIX ]] && + local suf=-S] -if [[ -n $functions[zsh_directory_name] || \ - ${+zsh_directory_name_functions} -ne 0 ]] ; then - [[ -n $functions[zsh_directory_name] ]] && zsh_directory_name c && ret=0 - for func in $zsh_directory_name_functions; do - $func c && ret=0 + _tags "$tag" + while _tags; do + while _next_label "$tag" expl "$descr"; do + expl+=( $suf ) + for func in $dirfuncs; do + $func c && ret=0 + done + done + (( ret )) || break done return ret else - _message 'dynamic directory name: implemented as zsh_directory_name c' + _message "${descr}: implement as zsh_directory_name c" fi diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo index 19f5909ea..6f86d0c54 100644 --- a/Doc/Zsh/expn.yo +++ b/Doc/Zsh/expn.yo @@ -2066,34 +2066,24 @@ tt(/home/pws/perforce). In this simple case a static name for the directory would be just as effective. example(zsh_directory_name+LPAR()RPAR() { - emulate -L zsh - setopt extendedglob + emulate -L zsh -o extendedglob local -a match mbegin mend - if [[ $1 = d ]]; then - # turn the directory into a name - if [[ $2 = (#b)(/home/pws/perforce/)([^/]##)* ]]; then - typeset -ga reply - reply=(p:$match[2] $(( ${#match[1]} + ${#match[2]} )) ) - else - return 1 - fi - elif [[ $1 = n ]]; then - # turn the name into a directory - [[ $2 != (#b)p:(?*) ]] && return 1 - typeset -ga reply - reply=(/home/pws/perforce/$match[1]) - elif [[ $1 = c ]]; then - # complete names - local expl - local -a dirs - dirs=(/home/pws/perforce/*(/:t)) - dirs=(p:${^dirs}) - _wanted dynamic-dirs expl 'dynamic directory' compadd -S\] -a dirs - return - else - return 1 - fi - return 0 + local base=/home/pws/perforce + case $1 in + ( d ) # Turn the directory into a name. + [[ $2 == (#b)($base/)([^/]##)* ]] && + reply=( p:$match[2] $(( $#match[1] + $#match[2] )) ) + ;; + ( n ) # Turn the name into a directory. + [[ $2 == (#b)p:(?*) ]] && + reply=( $base/$match[1] ) + ;; + ( c ) # Complete names. + local -a dirs=( $base/*(/:t) ) + # Completion system populates $expl with flags for compadd. + compadd "$expl[@]" p:$^dirs + ;; + esac }) texinode(Static named directories)(`=' expansion)(Dynamic named directories)(Filename Expansion) diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst index 51e246307..2524c43bd 100644 --- a/Test/Y01completion.ztst +++ b/Test/Y01completion.ztst @@ -93,6 +93,17 @@ >line: {: ~user2}{} >line: {: ~user1}{} + comptesteval 'zsh_directory_name() { compadd "$expl[@]" -- name1 name2 }' + comptest $': ~[\t\t\t\t' +0:dynamic directory names after ~[ +>line: {: ~[name}{} +>line: {: ~[name}{} +>DESCRIPTION:{dynamically named directory} +>NO:{name1} +>NO:{name2} +>line: {: ~[name1]}{} +>line: {: ~[name2]}{} + comptest $'echo ;:\C-b\C-b\t' 0:directories and files before separator >line: {echo }{;:} -- 2.39.2 (Apple Git-143) ^ permalink raw reply [flat|nested] 27+ messages in thread
* Patch 3/3: Fix subscript completion bugs inside ~[...] 2023-04-29 18:08 ` Patch 2/3: Make dynamic dir completion easier to implement Marlon Richert @ 2023-04-29 18:09 ` Marlon Richert 0 siblings, 0 replies; 27+ messages in thread From: Marlon Richert @ 2023-04-29 18:09 UTC (permalink / raw) To: Zsh hackers list [-- Attachment #1.1: Type: text/plain, Size: 2 bytes --] > [-- Attachment #1.2: Type: text/html, Size: 306 bytes --] [-- Attachment #2: 0003-Fix-subscript-completion-bugs-inside.txt --] [-- Type: text/plain, Size: 2797 bytes --] From 150b4cdac452e06a2ae38e5ba922f84636f2fcf7 Mon Sep 17 00:00:00 2001 From: Marlon Richert <marlon.richert@hibox.tv> Date: Sat, 29 Apr 2023 20:58:54 +0300 Subject: [PATCH 3/3] Fix subscript completion bugs inside ~[...] * _main_complete was exited prematurely, causing essential completion features, such as _setup and comppostfuncs, to be skipped. * Subscript completion was skipped when there were one or more slashes ('/') in the subscript. --- Completion/Base/Core/_main_complete | 22 +++++++++------------- Test/Y01completion.ztst | 16 +++++++++++++--- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete index 169ca1f40..408a66ee3 100644 --- a/Completion/Base/Core/_main_complete +++ b/Completion/Base/Core/_main_complete @@ -93,19 +93,15 @@ fi if [[ -z "$compstate[quote]" ]]; then if [[ -o equals ]] && compset -P 1 '='; then compstate[context]=equal - elif [[ "$PREFIX" != */* && "$PREFIX[1]" = '~' ]]; then - if [[ "$PREFIX" = '~['[^\]]# ]]; then - # Inside ~[...] should be treated as a subscript. - compset -p 2 - # To be consistent, we ignore all but the contents of the square - # brackets. - compset -S '\]*' - compstate[context]=subscript - [[ -n $_comps[-subscript-] ]] && $_comps[-subscript-] && return - else - compset -p 1 - compstate[context]=tilde - fi + elif [[ "$PREFIX" = \~\[[^]]# ]]; then + # Inside ~[...] should be treated as a subscript. + compset -p 2 + # To be consistent, we ignore all but the contents of the square brackets. + compset -S '\]*' + compstate[context]=subscript + elif [[ "$PREFIX" = \~[^/]# ]]; then + compset -p 1 + compstate[context]=tilde fi fi diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst index 2524c43bd..693ea7d58 100644 --- a/Test/Y01completion.ztst +++ b/Test/Y01completion.ztst @@ -93,17 +93,27 @@ >line: {: ~user2}{} >line: {: ~user1}{} - comptesteval 'zsh_directory_name() { compadd "$expl[@]" -- name1 name2 }' + comptesteval 'zsh_directory_name() { compadd "$expl[@]" -- name/1 name2 }' comptest $': ~[\t\t\t\t' 0:dynamic directory names after ~[ >line: {: ~[name}{} >line: {: ~[name}{} >DESCRIPTION:{dynamically named directory} ->NO:{name1} +>NO:{name/1} >NO:{name2} ->line: {: ~[name1]}{} +>line: {: ~[name/1]}{} >line: {: ~[name2]}{} + comptest $': ~[]\C-b\t\t\t\t' +0:dynamic directory names inside ~[...] +>line: {: ~[name}{]} +>line: {: ~[name}{]} +>DESCRIPTION:{dynamically named directory} +>NO:{name/1} +>NO:{name2} +>line: {: ~[name/1}{]} +>line: {: ~[name2}{]} + comptest $'echo ;:\C-b\C-b\t' 0:directories and files before separator >line: {echo }{;:} -- 2.39.2 (Apple Git-143) ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-04-29 18:07 Patch 1/3: Fix prefix insertion logic Marlon Richert 2023-04-29 18:08 ` Patch 2/3: Make dynamic dir completion easier to implement Marlon Richert @ 2023-04-29 18:58 ` Felipe Contreras 2023-04-29 19:02 ` Marlon Richert 1 sibling, 1 reply; 27+ messages in thread From: Felipe Contreras @ 2023-04-29 18:58 UTC (permalink / raw) To: Marlon Richert; +Cc: Zsh hackers list On Sat, Apr 29, 2023 at 1:09 PM Marlon Richert <marlon.richert@gmail.com> wrote: > > I don't know what's zsh's policy regarding sending patches, but personally I can't review zero content mails. -- Felipe Contreras ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-04-29 18:58 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras @ 2023-04-29 19:02 ` Marlon Richert 2023-04-29 19:17 ` Bart Schaefer ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Marlon Richert @ 2023-04-29 19:02 UTC (permalink / raw) To: Felipe Contreras; +Cc: Zsh hackers list On Sat, Apr 29, 2023 at 9:58 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > On Sat, Apr 29, 2023 at 1:09 PM Marlon Richert <marlon.richert@gmail.com> wrote: > I don't know what's zsh's policy regarding sending patches, but > personally I can't review zero content mails. Looks fine to me on the mailing list archives: https://www.zsh.org/mla/workers/2023/msg00409.html Should I copy-paste the commit message into the email's body next time? Would that help? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-04-29 19:02 ` Marlon Richert @ 2023-04-29 19:17 ` Bart Schaefer 2023-05-01 21:08 ` Aaron Schrab 2023-05-01 22:37 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras 2 siblings, 0 replies; 27+ messages in thread From: Bart Schaefer @ 2023-04-29 19:17 UTC (permalink / raw) To: Marlon Richert; +Cc: Felipe Contreras, Zsh hackers list On Sat, Apr 29, 2023 at 12:05 PM Marlon Richert <marlon.richert@gmail.com> wrote: > > Should I copy-paste the commit message into the email's body next time? > Would that help? That would be fine, but it would also be fine to just say "patch attached" or similar. Otherwise gmail previews as "(no body)" and the message might get ignored. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-04-29 19:02 ` Marlon Richert 2023-04-29 19:17 ` Bart Schaefer @ 2023-05-01 21:08 ` Aaron Schrab 2023-05-01 22:03 ` Bart Schaefer 2023-05-01 22:37 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras 2 siblings, 1 reply; 27+ messages in thread From: Aaron Schrab @ 2023-05-01 21:08 UTC (permalink / raw) To: Marlon Richert; +Cc: Felipe Contreras, Zsh hackers list At 22:02 +0300 29 Apr 2023, Marlon Richert <marlon.richert@gmail.com> wrote: >On Sat, Apr 29, 2023 at 9:58 PM Felipe Contreras ><felipe.contreras@gmail.com> wrote: >> On Sat, Apr 29, 2023 at 1:09 PM Marlon Richert <marlon.richert@gmail.com> wrote: >> I don't know what's zsh's policy regarding sending patches, but >> personally I can't review zero content mails. > >Looks fine to me on the mailing list archives: >https://www.zsh.org/mla/workers/2023/msg00409.html > >Should I copy-paste the commit message into the email's body next time? >Would that help? For the 1/3 and 2/3 there wasn't really a commit message other than the subject, the only content other than the patch itself was the git pseudo-header and stats about the files modified. I certainly don't find "Fix prefix insertion logic" to be a very helpful commit message. It might be a decent subject, but I'd expect the full message to give at least *some* idea of what the problem is. The second patch in the series had a subject that I'd consider slightly more useful ("Make dynamic dir completion easier to implement"), but I'd generally expect the description to give some idea of how it makes implementation easier. The final patch of the series does appear to have a decent message, although I haven't attempted to check how it lines up with the actual patch. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-01 21:08 ` Aaron Schrab @ 2023-05-01 22:03 ` Bart Schaefer 2023-05-05 11:41 ` [PATCH 1/3] Fix _prefix " Marlon Richert ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Bart Schaefer @ 2023-05-01 22:03 UTC (permalink / raw) To: Marlon Richert, Felipe Contreras, Zsh hackers list On Mon, May 1, 2023 at 2:10 PM Aaron Schrab <aaron@schrab.com> wrote: > > I certainly don't find "Fix prefix insertion logic" to be a very helpful > commit message. It might be a decent subject, but I'd expect the full > message to give at least *some* idea of what the problem is. Yes, if this connects back to an earlier thread ("Why is an 'x' appended ..." maybe?) then some reference to that should be made. > The second patch in the series had a subject that I'd consider slightly > more useful ("Make dynamic dir completion easier to implement"), but I'd > generally expect the description to give some idea of how it makes > implementation easier. Also, easier for whom? If the end user, I'd expect some sort of doc change, or at least a reference to what's currently difficult. > The final patch of the series does appear to have a decent message, > although I haven't attempted to check how it lines up with the actual > patch. This one looks OK, the included test cases appear to exemplify the issue. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/3] Fix _prefix insertion logic 2023-05-01 22:03 ` Bart Schaefer @ 2023-05-05 11:41 ` Marlon Richert 2023-06-07 6:03 ` Marlon Richert 2023-05-05 11:41 ` [PATCH 2/3] Make dynamic dir completion easier to implement Marlon Richert 2023-05-05 11:41 ` [PATCH 3/3] Fix subscript completion bugs inside ~[...] Marlon Richert 2 siblings, 1 reply; 27+ messages in thread From: Marlon Richert @ 2023-05-05 11:41 UTC (permalink / raw) To: zsh-workers; +Cc: Marlon Richert From: Marlon Richert <marlon.richert@gmail.com> This solves the following problems in the _prefix completer: - The old code had logic for dealing with compstate[unambiguous] that was unnecessary. It works fine without it. - Because of this logic, if a widget set compstate[insert]=1 after calling _main_complete, an `x` was left after the completion on the command line. - If the same widget also set `compstate[to_end]=`, then instead, the last character of the inserted completion would be treated as an autoremovable suffix, with the actual suffix being inserted to the line as a normal character. - After inserting a completion, the cursor would move to the end of the entire current word on the command, not the end of word that was inserted. This is not what you want with _prefix, since you are trying to complete a word _before_ the one on the command line, after which you usually want to insert a separator, such as a space or slash, before the next word. Discussed in workers/51641. --- Completion/Base/Completer/_prefix | 9 +-------- Test/Y01completion.ztst | 18 ++++++++++++++++++ Test/comptest | 2 +- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/Completion/Base/Completer/_prefix b/Completion/Base/Completer/_prefix index 74be5f47d..01739166e 100644 --- a/Completion/Base/Completer/_prefix +++ b/Completion/Base/Completer/_prefix @@ -49,14 +49,7 @@ for tmp in "$comp[@]"; do fi if [[ "$tmp" != _prefix ]] && "$tmp"; then - [[ compstate[nmatches] -gt 1 ]] && return 0 - compadd -U -i "$IPREFIX" -I "$ISUFFIX" - "${compstate[unambiguous]%$suf}x" - compstate[list]= - if [[ -n $compstate[unambiguous] ]]; then - compstate[insert]=unambiguous - else - compstate[insert]=0 - fi + compstate[to_end]='' return 0 fi (( _matcher_num++ )) diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst index f976f9f91..51e246307 100644 --- a/Test/Y01completion.ztst +++ b/Test/Y01completion.ztst @@ -63,6 +63,24 @@ >NO:{file1} >NO:{file2} + comptesteval 'tst-insert() { compstate[insert]=1; compstate[list]= }' + comptesteval 'comppostfuncs=( tst-insert )' + comptest $': dir1\ebf\t' +0:_prefix with compstate[insert]=1 does not move to end +>line: {: file1}{dir1} + + comptesteval 'unfunction tst-insert' + comptesteval 'comppostfuncs=()' + comptest $': dir1\ebf\t\t\t\t' +0:_prefix inserts unambiguous and does not move to end +>line: {: file}{dir1} +>line: {: file}{dir1} +>DESCRIPTION:{file} +>FI:{file1} +>FI:{file2} +>line: {: file1}{dir1} +>line: {: file2}{dir1} + comptesteval $'zstyle -d \'*\' glob' comptesteval '_users () { compadd user1 user2 }' comptest $': ~\t\t\t\t\t' diff --git a/Test/comptest b/Test/comptest index 79c69979a..a57f4bcc4 100644 --- a/Test/comptest +++ b/Test/comptest @@ -40,7 +40,7 @@ KEYTIMEOUT=1 setopt zle autoload -U compinit compinit -u -zstyle ":completion:*" completer _expand _complete _ignored +zstyle ":completion:*" completer _expand _complete _prefix _ignored zstyle ":completion:*:default" list-colors "no=<NO>" "fi=<FI>" "di=<DI>" "ln=<LN>" "pi=<PI>" "so=<SO>" "bd=<BD>" "cd=<CD>" "ex=<EX>" "mi=<MI>" "tc=<TC>" "sp=<SP>" "lc=<LC>" "ec=<EC>\n" "rc=<RC>" zstyle ":completion:*" group-name "" zstyle ":completion:*:messages" format "<MESSAGE>%d</MESSAGE> -- 2.39.2 (Apple Git-143) ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] Fix _prefix insertion logic 2023-05-05 11:41 ` [PATCH 1/3] Fix _prefix " Marlon Richert @ 2023-06-07 6:03 ` Marlon Richert 2023-06-08 12:41 ` Jun. T 2023-06-15 14:11 ` Marlon Richert 0 siblings, 2 replies; 27+ messages in thread From: Marlon Richert @ 2023-06-07 6:03 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers On Fri, May 5, 2023 at 2:44 PM Marlon Richert <marlon.richert@gmail.com> wrote: > > This solves the following problems in the _prefix completer: > - The old code had logic for dealing with compstate[unambiguous] that > was unnecessary. It works fine without it. > - Because of this logic, if a widget set compstate[insert]=1 after > calling _main_complete, an `x` was left after the completion on the > command line. > - If the same widget also set `compstate[to_end]=`, then instead, the > last character of the inserted completion would be treated as an > autoremovable suffix, with the actual suffix being inserted to the > line as a normal character. > - After inserting a completion, the cursor would move to the end of the > entire current word on the command, not the end of word that was > inserted. This is not what you want with _prefix, since you are trying > to complete a word _before_ the one on the command line, after which > you usually want to insert a separator, such as a space or slash, > before the next word. > > Discussed in workers/51641. > --- Any further comments on this patch? Will it be committed? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] Fix _prefix insertion logic 2023-06-07 6:03 ` Marlon Richert @ 2023-06-08 12:41 ` Jun. T 2023-06-15 14:11 ` Marlon Richert 1 sibling, 0 replies; 27+ messages in thread From: Jun. T @ 2023-06-08 12:41 UTC (permalink / raw) To: zsh-workers > 2023/06/07 15:03, Marlon Richert <marlon.richert@gmail.com> wrote: > > On Fri, May 5, 2023 at 2:44 PM Marlon Richert <marlon.richert@gmail.com> wrote: >> >> >> Discussed in workers/51641. > > Any further comments on this patch? Will it be committed? I haven't yet tried to understand the patch, but if I test the Case 1 and 2 in workers/51641 with the [PATCH 1/3] in 51716: Case 1: % file f<TAB>bar % file foo/bar but hitting SPACE gives: % file fo /bar Case 2: % file b<TAB>foo % file bar/foo but again hitting SPACE gives: % file ba /foo Or am I missing something? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] Fix _prefix insertion logic 2023-06-07 6:03 ` Marlon Richert 2023-06-08 12:41 ` Jun. T @ 2023-06-15 14:11 ` Marlon Richert 1 sibling, 0 replies; 27+ messages in thread From: Marlon Richert @ 2023-06-15 14:11 UTC (permalink / raw) To: Jun . T; +Cc: zsh-workers You're right, the test cases still fail. I added test cases to the build and wrote code until they past. However, I had bugs in the test cases and thus ended up not solving the problem. Here's a new patch, below. I added more exhaustive testing to make sure it definitely works correctly now. --- Fix _prefix insertion logic This solves the following problems in the _prefix completer: - The old code had logic for dealing with compstate[unambiguous] that was unnecessary. It works fine without it. - Because of this logic, if a widget set compstate[insert]=1 after calling _main_complete, an `x` was left after the completion on the command line. - If the same widget also set `compstate[to_end]=`, then instead, the last character of the inserted completion would be treated as an autoremovable suffix, with the actual suffix being inserted to the line as a normal character. - After inserting a completion, the cursor would move to the end of the entire current word on the command, not the end of word that was inserted. This is not what you want with _prefix, since you are trying to complete a word _before_ the one on the command line, after which you usually want to insert a separator, such as a space or slash, before the next word. Discussed in workers/51641. --- Completion/Base/Completer/_prefix | 9 ++---- Test/Y01completion.ztst | 48 +++++++++++++++++++++++++++++++ Test/comptest | 10 +++++-- 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/Completion/Base/Completer/_prefix b/Completion/Base/Completer/_prefix index 74be5f47d..aea2f7863 100644 --- a/Completion/Base/Completer/_prefix +++ b/Completion/Base/Completer/_prefix @@ -49,13 +49,8 @@ for tmp in "$comp[@]"; do fi if [[ "$tmp" != _prefix ]] && "$tmp"; then - [[ compstate[nmatches] -gt 1 ]] && return 0 - compadd -U -i "$IPREFIX" -I "$ISUFFIX" - "${compstate[unambiguous]%$suf}x" - compstate[list]= - if [[ -n $compstate[unambiguous] ]]; then - compstate[insert]=unambiguous - else - compstate[insert]=0 + if [[ -n $compstate[old_list] || ${compstate[unambiguous]%$suf} == $PREFIX ]]; then + compstate[to_end]=match fi return 0 fi diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst index fb369ea69..fc18b19a4 100644 --- a/Test/Y01completion.ztst +++ b/Test/Y01completion.ztst @@ -35,6 +35,54 @@ >line: {: dir1/}{} >line: {: dir2/}{} + comptest $': d\t\t\t\t\t \t' +0:unambiguous prefix and autoremovable suffix +>line: {: dir}{} +>line: {: dir}{} +>DESCRIPTION:{file} +>DI:{dir1} +>DI:{dir2} +>line: {: dir1/}{} +>line: {: dir2/}{} +>line: {: dir1/}{} +>line: {: dir1 }{} +>DESCRIPTION:{file} +>DI:{dir1} +>DI:{dir2} +>FI:{file1} +>FI:{file2} + + comptest $': suf\ebd\t\t\t\t\t \t' +0:unambiguous prefix and autoremovable suffix with _prefix completer +>line: {: dir}{suf} +>line: {: dir}{suf} +>DESCRIPTION:{file} +>DI:{dir1} +>DI:{dir2} +>line: {: dir1/}{suf} +>line: {: dir2/}{suf} +>line: {: dir1/}{suf} +>line: {: dir1 }{suf} +>DESCRIPTION:{file} +>DI:{dir1} +>DI:{dir2} +>FI:{file1} +>FI:{file2} +F:regression test workers/51641 + + comptesteval 'comptest-postfunc() { compstate[insert]=1 compstate[list]= }' + comptest $': \t \t' +0:compstate[insert]=1 compstate[list]= +>line: {: dir1/}{} +>line: {: dir1 dir1/}{} + + comptest $': suf\eb\t \t' +0:compstate[insert]=1 compstate[list]= with _prefix completer +>line: {: dir1/}{suf} +>line: {: dir1 dir1/}{suf} +F:regression test workers/51641 + + comptesteval 'comptest-postfunc() {}' comptest $': *\t\t\t\t\t\t' 0:_expand shows file types >line: {: dir1/}{} diff --git a/Test/comptest b/Test/comptest index 79c69979a..4a9bd9b7f 100644 --- a/Test/comptest +++ b/Test/comptest @@ -21,7 +21,7 @@ comptestinit () { export PS1="<PROMPT>" zpty zsh "$comptest_zsh -f +Z" - zpty -r zsh log1 "*<PROMPT>*" || { + zpty -r zsh log1 "*<PROMPT>*" || { print "first prompt hasn't appeared." return 1 } @@ -40,7 +40,7 @@ KEYTIMEOUT=1 setopt zle autoload -U compinit compinit -u -zstyle ":completion:*" completer _expand _complete _ignored +zstyle ":completion:*" completer _expand _complete _prefix _ignored zstyle ":completion:*:default" list-colors "no=<NO>" "fi=<FI>" "di=<DI>" "ln=<LN>" "pi=<PI>" "so=<SO>" "bd=<BD>" "cd=<CD>" "ex=<EX>" "mi=<MI>" "tc=<TC>" "sp=<SP>" "lc=<LC>" "ec=<EC>\n" "rc=<RC>" zstyle ":completion:*" group-name "" zstyle ":completion:*:messages" format "<MESSAGE>%d</MESSAGE> @@ -51,6 +51,12 @@ zstyle ":completion:*:options" verbose yes zstyle ":completion:*:values" verbose yes setopt noalwayslastprompt listrowsfirst completeinword zmodload zsh/complist +zle -C complete-word complete-word complete-word-with-postfunc +complete-word-with-postfunc() { + local +h -a comppostfuncs=( comptest-postfunc ) + _main_complete "$@" +} +comptest-postfunc() {} complete-word-with-report () { print -lr "<WIDGET><complete-word>" zle complete-word -- 2.39.2 (Apple Git-143) ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/3] Make dynamic dir completion easier to implement 2023-05-01 22:03 ` Bart Schaefer 2023-05-05 11:41 ` [PATCH 1/3] Fix _prefix " Marlon Richert @ 2023-05-05 11:41 ` Marlon Richert 2023-05-06 17:28 ` Bart Schaefer 2023-05-15 9:04 ` Oliver Kiddle 2023-05-05 11:41 ` [PATCH 3/3] Fix subscript completion bugs inside ~[...] Marlon Richert 2 siblings, 2 replies; 27+ messages in thread From: Marlon Richert @ 2023-05-05 11:41 UTC (permalink / raw) To: zsh-workers; +Cc: Marlon Richert From: Marlon Richert <marlon.richert@gmail.com> In the pre-patch example, the recommended way of implementing this was as follows: _wanted dynamic-dirs expl 'dynamic directory' compadd -S\] ... However, this is problematic for the following reasons: - The proper place for handling _wanted or other tag logic is in the completion function _dynamic_directory_name, not in each dynamic dir name function separately. - The naming styles of the tag and group are not consistent with those used for non-dynamic named directories, namely 'named-directories' and 'named directory', respectively. - The example always inserted a `]` suffix, even if the suffix was already present on the command line, leading to a double `]]` on the command line. - Code addressing the points the above needed to be duplicated in each dynamic dir name function. This patch fixes these problems by - moving the completion logic from the example into _dynamic_directory_name, so that users don't have to duplicated this scaffolding for each dynamic dir name function, and - adds logic to _dynamic_directory_name for inserting the `]` suffix only when needed. --- .../Zsh/Context/_dynamic_directory_name | 30 +++++++++---- Doc/Zsh/expn.yo | 44 +++++++------------ Test/Y01completion.ztst | 11 +++++ 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/Completion/Zsh/Context/_dynamic_directory_name b/Completion/Zsh/Context/_dynamic_directory_name index f449c3b12..e173dcb7f 100644 --- a/Completion/Zsh/Context/_dynamic_directory_name +++ b/Completion/Zsh/Context/_dynamic_directory_name @@ -1,15 +1,29 @@ #autoload +local -a dirfuncs=( + ${(k)functions[zsh_directory_name]} + $zsh_directory_name_functions +) +local descr='dynamically named directory' -local func -integer ret=1 +if (( $#dirfuncs )); then + local -a expl=() + local -i ret=1 + local func= + local tag=dynamically-named-directories + [[ -z $ISUFFIX ]] && + local suf=-S] -if [[ -n $functions[zsh_directory_name] || \ - ${+zsh_directory_name_functions} -ne 0 ]] ; then - [[ -n $functions[zsh_directory_name] ]] && zsh_directory_name c && ret=0 - for func in $zsh_directory_name_functions; do - $func c && ret=0 + _tags "$tag" + while _tags; do + while _next_label "$tag" expl "$descr"; do + expl+=( $suf ) + for func in $dirfuncs; do + $func c && ret=0 + done + done + (( ret )) || break done return ret else - _message 'dynamic directory name: implemented as zsh_directory_name c' + _message "${descr}: implement as zsh_directory_name c" fi diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo index 19f5909ea..6f86d0c54 100644 --- a/Doc/Zsh/expn.yo +++ b/Doc/Zsh/expn.yo @@ -2066,34 +2066,24 @@ tt(/home/pws/perforce). In this simple case a static name for the directory would be just as effective. example(zsh_directory_name+LPAR()RPAR() { - emulate -L zsh - setopt extendedglob + emulate -L zsh -o extendedglob local -a match mbegin mend - if [[ $1 = d ]]; then - # turn the directory into a name - if [[ $2 = (#b)(/home/pws/perforce/)([^/]##)* ]]; then - typeset -ga reply - reply=(p:$match[2] $(( ${#match[1]} + ${#match[2]} )) ) - else - return 1 - fi - elif [[ $1 = n ]]; then - # turn the name into a directory - [[ $2 != (#b)p:(?*) ]] && return 1 - typeset -ga reply - reply=(/home/pws/perforce/$match[1]) - elif [[ $1 = c ]]; then - # complete names - local expl - local -a dirs - dirs=(/home/pws/perforce/*(/:t)) - dirs=(p:${^dirs}) - _wanted dynamic-dirs expl 'dynamic directory' compadd -S\] -a dirs - return - else - return 1 - fi - return 0 + local base=/home/pws/perforce + case $1 in + ( d ) # Turn the directory into a name. + [[ $2 == (#b)($base/)([^/]##)* ]] && + reply=( p:$match[2] $(( $#match[1] + $#match[2] )) ) + ;; + ( n ) # Turn the name into a directory. + [[ $2 == (#b)p:(?*) ]] && + reply=( $base/$match[1] ) + ;; + ( c ) # Complete names. + local -a dirs=( $base/*(/:t) ) + # Completion system populates $expl with flags for compadd. + compadd "$expl[@]" p:$^dirs + ;; + esac }) texinode(Static named directories)(`=' expansion)(Dynamic named directories)(Filename Expansion) diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst index 51e246307..2524c43bd 100644 --- a/Test/Y01completion.ztst +++ b/Test/Y01completion.ztst @@ -93,6 +93,17 @@ >line: {: ~user2}{} >line: {: ~user1}{} + comptesteval 'zsh_directory_name() { compadd "$expl[@]" -- name1 name2 }' + comptest $': ~[\t\t\t\t' +0:dynamic directory names after ~[ +>line: {: ~[name}{} +>line: {: ~[name}{} +>DESCRIPTION:{dynamically named directory} +>NO:{name1} +>NO:{name2} +>line: {: ~[name1]}{} +>line: {: ~[name2]}{} + comptest $'echo ;:\C-b\C-b\t' 0:directories and files before separator >line: {echo }{;:} -- 2.39.2 (Apple Git-143) ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] Make dynamic dir completion easier to implement 2023-05-05 11:41 ` [PATCH 2/3] Make dynamic dir completion easier to implement Marlon Richert @ 2023-05-06 17:28 ` Bart Schaefer 2023-05-13 17:30 ` Bart Schaefer 2023-05-15 9:04 ` Oliver Kiddle 1 sibling, 1 reply; 27+ messages in thread From: Bart Schaefer @ 2023-05-06 17:28 UTC (permalink / raw) To: zsh-workers; +Cc: Marlon Richert Not necessarily asking Marlon to do this, but if this patch is going to be included we need to repair these two functions: Functions/Chpwd/zsh_directory_name_cdr Functions/Chpwd/zsh_directory_name_generic ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] Make dynamic dir completion easier to implement 2023-05-06 17:28 ` Bart Schaefer @ 2023-05-13 17:30 ` Bart Schaefer 0 siblings, 0 replies; 27+ messages in thread From: Bart Schaefer @ 2023-05-13 17:30 UTC (permalink / raw) To: zsh-workers On Sat, May 6, 2023 at 10:28 AM Bart Schaefer <schaefer@brasslantern.com> wrote: > > Not necessarily asking Marlon to do this, but if this patch is going > to be included we need to repair these two functions: > > Functions/Chpwd/zsh_directory_name_cdr > Functions/Chpwd/zsh_directory_name_generic Attempting to extrapolate from Marlon's edits to the doc example, I think the only part of these that really needs to change is the "if completion" branch. However, I'm not a user of the "cdr" functions so I'm not sure what to do with the former, or if the "Can't complete in the middle" warning still applies in the latter (and if it does, is that a problem with Marlon's patch?); nor if the explanatory text passed to _describe in either function has to move somewhere. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] Make dynamic dir completion easier to implement 2023-05-05 11:41 ` [PATCH 2/3] Make dynamic dir completion easier to implement Marlon Richert 2023-05-06 17:28 ` Bart Schaefer @ 2023-05-15 9:04 ` Oliver Kiddle 1 sibling, 0 replies; 27+ messages in thread From: Oliver Kiddle @ 2023-05-15 9:04 UTC (permalink / raw) To: Marlon Richert; +Cc: zsh-workers Sorry for not reviewing this sooner. Thanks for following up to Aaron's reply with a bit more detail as that does help. On 5 May, Marlon Richert wrote: > > In the pre-patch example, the recommended way of implementing this was > as follows: > _wanted dynamic-dirs expl 'dynamic directory' compadd -S\] ... > > However, this is problematic for the following reasons: > - The proper place for handling _wanted or other tag logic is in the > completion function _dynamic_directory_name, not in each dynamic dir > name function separately. That's questionable. Adding a tag loop outside as in your patch probably makes it easier for someone to setup dynamic directories. It does simplify the example. And it doesn't stop people using their own tag loop - that can be necessary where the dynamic names are broken down into components for the purposes of completion. So I've no objection to the change. There are aspects of having nested tag loops that isn't ideal but it's better than no tag loop. > - The naming styles of the tag and group are not consistent with those > used for non-dynamic named directories, namely 'named-directories' and > 'named directory', respectively. That's specific to the example in the documentation. For many real cases, the text used for dynamic directories can be more usefully described in some other way. Such as 'recent directory index' in the cdr example. > - The example always inserted a `]` suffix, even if the suffix was > already present on the command line, leading to a double `]]` on the > command line. Yes, that's not good. And passing the suffix to the function as a parameter is a good approach to take. > -local func > -integer ret=1 > +if (( $#dirfuncs )); then > + local -a expl=() > + local -i ret=1 > + local func= There's no need to explicitly assign empty values. The function isn't even relying on the initial value and local does reset the value to be empty anyway. > + local tag=dynamically-named-directories > + [[ -z $ISUFFIX ]] && > + local suf=-S] This will only declare the variable suf local if the condition evaluates to true. If it is false, we could be picking an unwanted value and pass it to compadd. Aside from that, I don't think the condition is correct because the user could be completing in the middle of a word. zsh_directory_name_cdr uses [[ $ISUFFIX = *\]* ]] || addsuffix='-S]' Within _dynamic_directory_name, I think it should be just [[ $ISUFFIX = \]* ]] > -if [[ -n $functions[zsh_directory_name] || \ > - ${+zsh_directory_name_functions} -ne 0 ]] ; then > - [[ -n $functions[zsh_directory_name] ]] && zsh_directory_name c && ret=0 > - for func in $zsh_directory_name_functions; do > - $func c && ret=0 > + _tags "$tag" > + while _tags; do > + while _next_label "$tag" expl "$descr"; do > + expl+=( $suf ) You can simply add $suf to the end of the call to _next_label and it will add extra options to $expl: while _next_label "$tag" expl "$descr" $suf; do The cdr functions that Bart mentioned need their own descriptions but at least zsh_directory_name_cdr could be tweaked to take advantage of the suffix now being passed to it. I don't see this breaking any existing dynamic directory functions which was my initial worry before looking at the changes more closely. Oliver ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/3] Fix subscript completion bugs inside ~[...] 2023-05-01 22:03 ` Bart Schaefer 2023-05-05 11:41 ` [PATCH 1/3] Fix _prefix " Marlon Richert 2023-05-05 11:41 ` [PATCH 2/3] Make dynamic dir completion easier to implement Marlon Richert @ 2023-05-05 11:41 ` Marlon Richert 2 siblings, 0 replies; 27+ messages in thread From: Marlon Richert @ 2023-05-05 11:41 UTC (permalink / raw) To: zsh-workers; +Cc: Marlon Richert From: Marlon Richert <marlon.richert@gmail.com> When completing inside ~[...] (_with_ the trailing `]` present), the following bugs occured: - Subscript completion was skipped entirely when there were one or more slashes ('/') in the subscript, which is incorrect, since slashes are allowed there. - Instead of going through _complete, $_comps[-subscript-] was called immediately, causing _setup to be skipped. - If succesful, _main_complete was exited right after, causing menu-style, comppostfuncs and other essential completion features to be skipped. --- Completion/Base/Core/_main_complete | 22 +++++++++------------- Test/Y01completion.ztst | 16 +++++++++++++--- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete index 169ca1f40..408a66ee3 100644 --- a/Completion/Base/Core/_main_complete +++ b/Completion/Base/Core/_main_complete @@ -93,19 +93,15 @@ fi if [[ -z "$compstate[quote]" ]]; then if [[ -o equals ]] && compset -P 1 '='; then compstate[context]=equal - elif [[ "$PREFIX" != */* && "$PREFIX[1]" = '~' ]]; then - if [[ "$PREFIX" = '~['[^\]]# ]]; then - # Inside ~[...] should be treated as a subscript. - compset -p 2 - # To be consistent, we ignore all but the contents of the square - # brackets. - compset -S '\]*' - compstate[context]=subscript - [[ -n $_comps[-subscript-] ]] && $_comps[-subscript-] && return - else - compset -p 1 - compstate[context]=tilde - fi + elif [[ "$PREFIX" = \~\[[^]]# ]]; then + # Inside ~[...] should be treated as a subscript. + compset -p 2 + # To be consistent, we ignore all but the contents of the square brackets. + compset -S '\]*' + compstate[context]=subscript + elif [[ "$PREFIX" = \~[^/]# ]]; then + compset -p 1 + compstate[context]=tilde fi fi diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst index 2524c43bd..693ea7d58 100644 --- a/Test/Y01completion.ztst +++ b/Test/Y01completion.ztst @@ -93,17 +93,27 @@ >line: {: ~user2}{} >line: {: ~user1}{} - comptesteval 'zsh_directory_name() { compadd "$expl[@]" -- name1 name2 }' + comptesteval 'zsh_directory_name() { compadd "$expl[@]" -- name/1 name2 }' comptest $': ~[\t\t\t\t' 0:dynamic directory names after ~[ >line: {: ~[name}{} >line: {: ~[name}{} >DESCRIPTION:{dynamically named directory} ->NO:{name1} +>NO:{name/1} >NO:{name2} ->line: {: ~[name1]}{} +>line: {: ~[name/1]}{} >line: {: ~[name2]}{} + comptest $': ~[]\C-b\t\t\t\t' +0:dynamic directory names inside ~[...] +>line: {: ~[name}{]} +>line: {: ~[name}{]} +>DESCRIPTION:{dynamically named directory} +>NO:{name/1} +>NO:{name2} +>line: {: ~[name/1}{]} +>line: {: ~[name2}{]} + comptest $'echo ;:\C-b\C-b\t' 0:directories and files before separator >line: {echo }{;:} -- 2.39.2 (Apple Git-143) ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-04-29 19:02 ` Marlon Richert 2023-04-29 19:17 ` Bart Schaefer 2023-05-01 21:08 ` Aaron Schrab @ 2023-05-01 22:37 ` Felipe Contreras 2023-05-01 22:52 ` Bart Schaefer 2023-05-01 23:36 ` Vin Shelton 2 siblings, 2 replies; 27+ messages in thread From: Felipe Contreras @ 2023-05-01 22:37 UTC (permalink / raw) To: Marlon Richert; +Cc: Zsh hackers list On Sat, Apr 29, 2023 at 2:03 PM Marlon Richert <marlon.richert@gmail.com> wrote: > > On Sat, Apr 29, 2023 at 9:58 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > On Sat, Apr 29, 2023 at 1:09 PM Marlon Richert <marlon.richert@gmail.com> wrote: > > I don't know what's zsh's policy regarding sending patches, but > > personally I can't review zero content mails. > > Looks fine to me on the mailing list archives: > https://www.zsh.org/mla/workers/2023/msg00409.html Yes, and I suspect in many mail clients as well. But not in Gmail. A standard way to send patches is with `git send-email` which sends the mails generated by `git format-patch` as is. Notice that your attachments have "From", "Date" and "Subject" mail fields, so they can be piped to sendmail. > Should I copy-paste the commit message into the email's body next time? I would recommend to configure `git sendemail` as sending patches to any mailing list becomes much easier. But again, that's my personal recommendation, I don't know if there's any zsh guideline. -- Felipe Contreras ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-01 22:37 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras @ 2023-05-01 22:52 ` Bart Schaefer 2023-05-02 4:49 ` Felipe Contreras 2023-05-01 23:36 ` Vin Shelton 1 sibling, 1 reply; 27+ messages in thread From: Bart Schaefer @ 2023-05-01 22:52 UTC (permalink / raw) To: Felipe Contreras; +Cc: Marlon Richert, Zsh hackers list On Mon, May 1, 2023 at 3:38 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > I would recommend to configure `git sendemail` as sending patches to > any mailing list becomes much easier. This requires a recent enough version of git (2.25 bundled with Ubuntu 20.04 doesn't have it, for example) and either a sendmail-like command or the ability to configure an SMTP server connection. > But again, that's my personal recommendation, I don't know if there's > any zsh guideline. If you can do the above git stuff, great. If instead you must rely on putting the diffs through an email client: 1) If the lines won't be wrapped or converted to HTML or otherwise re-padded, copy directly into the email body. 2) Otherwise, attach as type text/plain (force ".txt" extension if necessary). Please NOT as ".diff" or ".patch" or text/x-something. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-01 22:52 ` Bart Schaefer @ 2023-05-02 4:49 ` Felipe Contreras 2023-05-02 19:16 ` Bart Schaefer 2023-05-05 11:51 ` Marlon Richert 0 siblings, 2 replies; 27+ messages in thread From: Felipe Contreras @ 2023-05-02 4:49 UTC (permalink / raw) To: Bart Schaefer; +Cc: Marlon Richert, Zsh hackers list On Mon, May 1, 2023 at 5:52 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > On Mon, May 1, 2023 at 3:38 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > I would recommend to configure `git sendemail` as sending patches to > > any mailing list becomes much easier. > > This requires a recent enough version of git No, it has always been there, even before 1.0 in 2005. > (2.25 bundled with Ubuntu 20.04 doesn't have it, for example) It's right there: https://packages.ubuntu.com/focal/all/git-email/filelist > and either a sendmail-like command or the ability to configure an SMTP > server connection. Yeah, which all servers I've ever used support, including Gmail. `git send-email --help` explains how to configure Gmail as an SMTP server [1]. Although personally I simply use msmtp [2], which works for other programs and uses my keyring. [1] https://git-scm.com/docs/git-send-email [2] https://marlam.de/msmtp/ -- Felipe Contreras ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-02 4:49 ` Felipe Contreras @ 2023-05-02 19:16 ` Bart Schaefer 2023-05-02 20:14 ` Felipe Contreras 2023-05-05 11:51 ` Marlon Richert 1 sibling, 1 reply; 27+ messages in thread From: Bart Schaefer @ 2023-05-02 19:16 UTC (permalink / raw) To: Felipe Contreras; +Cc: Marlon Richert, Zsh hackers list On Mon, May 1, 2023 at 9:49 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > On Mon, May 1, 2023 at 5:52 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > On Mon, May 1, 2023 at 3:38 PM Felipe Contreras > > <felipe.contreras@gmail.com> wrote: > > > I would recommend to configure `git sendemail` as sending patches to > > > any mailing list becomes much easier. > > > > This requires a recent enough version of git > > No, it has always been there, even before 1.0 in 2005. Aha, I wasn't aware that Ubuntu requires it to be installed as a separate package. It's not present if only the "git" package is installed. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-02 19:16 ` Bart Schaefer @ 2023-05-02 20:14 ` Felipe Contreras 0 siblings, 0 replies; 27+ messages in thread From: Felipe Contreras @ 2023-05-02 20:14 UTC (permalink / raw) To: Bart Schaefer; +Cc: Marlon Richert, Zsh hackers list On Tue, May 2, 2023 at 2:16 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > On Mon, May 1, 2023 at 9:49 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > On Mon, May 1, 2023 at 5:52 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > > On Mon, May 1, 2023 at 3:38 PM Felipe Contreras > > > <felipe.contreras@gmail.com> wrote: > > > > I would recommend to configure `git sendemail` as sending patches to > > > > any mailing list becomes much easier. > > > > > > This requires a recent enough version of git > > > > No, it has always been there, even before 1.0 in 2005. > > Aha, I wasn't aware that Ubuntu requires it to be installed as a > separate package. It's not present if only the "git" package is > installed. I think it's Debian. I'm not sure why they do that, but I suspect it is to split the dependencies on certain perl libraries specific for `git send-email`, but they are optional. -- Felipe Contreras ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-02 4:49 ` Felipe Contreras 2023-05-02 19:16 ` Bart Schaefer @ 2023-05-05 11:51 ` Marlon Richert 2023-05-05 12:38 ` Felipe Contreras 2023-05-05 15:04 ` Mikael Magnusson 1 sibling, 2 replies; 27+ messages in thread From: Marlon Richert @ 2023-05-05 11:51 UTC (permalink / raw) To: Felipe Contreras; +Cc: Bart Schaefer, Zsh hackers list On Tue, May 2, 2023 at 7:49 AM Felipe Contreras <felipe.contreras@gmail.com> wrote: > Yeah, which all servers I've ever used support, including Gmail. `git > send-email --help` explains how to configure Gmail as an SMTP server Thanks for the feedback and tips! I used `git send-email` this time. However even though I used the same message ID, the new mails did not end up in the same thread. Not sure what's up with that. Should I have excluded the enclosing `<` and `>`? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-05 11:51 ` Marlon Richert @ 2023-05-05 12:38 ` Felipe Contreras 2023-05-05 15:04 ` Mikael Magnusson 1 sibling, 0 replies; 27+ messages in thread From: Felipe Contreras @ 2023-05-05 12:38 UTC (permalink / raw) To: Marlon Richert; +Cc: Bart Schaefer, Zsh hackers list On Fri, May 5, 2023 at 6:51 AM Marlon Richert <marlon.richert@gmail.com> wrote: > > On Tue, May 2, 2023 at 7:49 AM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > Yeah, which all servers I've ever used support, including Gmail. `git > > send-email --help` explains how to configure Gmail as an SMTP server > > Thanks for the feedback and tips! I used `git send-email` this time. > However even though I used the same message ID, the new mails did not > end up in the same thread. Not sure what's up with that. Should I have > excluded the enclosing `<` and `>`? I do see them in the same thread in reply to the mid you specified: https://www.zsh.org/mla/workers/2023/msg00423.html You don't need to add '<>', I do it like: --in-reply-to=20230503052926.217219-1-felipe.contreras@gmail.com Cheers. -- Felipe Contreras ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-05 11:51 ` Marlon Richert 2023-05-05 12:38 ` Felipe Contreras @ 2023-05-05 15:04 ` Mikael Magnusson 2023-05-06 17:23 ` Bart Schaefer 1 sibling, 1 reply; 27+ messages in thread From: Mikael Magnusson @ 2023-05-05 15:04 UTC (permalink / raw) To: Marlon Richert; +Cc: Zsh hackers list On 5/5/23, Marlon Richert <marlon.richert@gmail.com> wrote: > On Tue, May 2, 2023 at 7:49 AM Felipe Contreras > <felipe.contreras@gmail.com> wrote: >> Yeah, which all servers I've ever used support, including Gmail. `git >> send-email --help` explains how to configure Gmail as an SMTP server > > Thanks for the feedback and tips! I used `git send-email` this time. > However even though I used the same message ID, the new mails did not > end up in the same thread. Not sure what's up with that. Should I have > excluded the enclosing `<` and `>`? If you're looking in the gmail web interface, the reason is that gmail doesn't support threads (it ignores all thread information and only groups mails with the same subject line). -- Mikael Magnusson ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-05 15:04 ` Mikael Magnusson @ 2023-05-06 17:23 ` Bart Schaefer 0 siblings, 0 replies; 27+ messages in thread From: Bart Schaefer @ 2023-05-06 17:23 UTC (permalink / raw) To: Mikael Magnusson; +Cc: Marlon Richert, Zsh hackers list On Fri, May 5, 2023 at 8:04 AM Mikael Magnusson <mikachu@gmail.com> wrote: > > On 5/5/23, Marlon Richert <marlon.richert@gmail.com> wrote: > > However even though I used the same message ID, the new mails did not > > end up in the same thread. > > If you're looking in the gmail web interface, the reason is that gmail > doesn't support threads (it ignores all thread information and only > groups mails with the same subject line). The one exception to this is that gmail will treat messages with identical message-id as duplicates and only show one of them. That's why e.g. if you send a message to the list using gmail it'll appear only in your Sent label and seem not to have come back to you via the list. I'm not sure what would happen if it were presented with identical message-id and radically different message bodies, but in my experience small differences in the body do not affect this. Looks like something (git send-email maybe?) appended -1 / -2 / -3 to the message-id in each of these patches, which are otherwise the same. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Patch 1/3: Fix prefix insertion logic 2023-05-01 22:37 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras 2023-05-01 22:52 ` Bart Schaefer @ 2023-05-01 23:36 ` Vin Shelton 1 sibling, 0 replies; 27+ messages in thread From: Vin Shelton @ 2023-05-01 23:36 UTC (permalink / raw) To: Felipe Contreras; +Cc: Marlon Richert, Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 782 bytes --] On Mon, May 1, 2023 at 6:38 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > On Sat, Apr 29, 2023 at 2:03 PM Marlon Richert <marlon.richert@gmail.com> > wrote: > > > > On Sat, Apr 29, 2023 at 9:58 PM Felipe Contreras > > <felipe.contreras@gmail.com> wrote: > > > On Sat, Apr 29, 2023 at 1:09 PM Marlon Richert < > marlon.richert@gmail.com> wrote: > > > I don't know what's zsh's policy regarding sending patches, but > > > personally I can't review zero content mails. > > > > Looks fine to me on the mailing list archives: > > https://www.zsh.org/mla/workers/2023/msg00409.html > > Yes, and I suspect in many mail clients as well. But not in Gmail. > Odd. I use gmail and the first patch came through as an attachment for me. HTH, Vin [-- Attachment #2: Type: text/html, Size: 2033 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-06-15 14:12 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-29 18:07 Patch 1/3: Fix prefix insertion logic Marlon Richert 2023-04-29 18:08 ` Patch 2/3: Make dynamic dir completion easier to implement Marlon Richert 2023-04-29 18:09 ` Patch 3/3: Fix subscript completion bugs inside ~[...] Marlon Richert 2023-04-29 18:58 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras 2023-04-29 19:02 ` Marlon Richert 2023-04-29 19:17 ` Bart Schaefer 2023-05-01 21:08 ` Aaron Schrab 2023-05-01 22:03 ` Bart Schaefer 2023-05-05 11:41 ` [PATCH 1/3] Fix _prefix " Marlon Richert 2023-06-07 6:03 ` Marlon Richert 2023-06-08 12:41 ` Jun. T 2023-06-15 14:11 ` Marlon Richert 2023-05-05 11:41 ` [PATCH 2/3] Make dynamic dir completion easier to implement Marlon Richert 2023-05-06 17:28 ` Bart Schaefer 2023-05-13 17:30 ` Bart Schaefer 2023-05-15 9:04 ` Oliver Kiddle 2023-05-05 11:41 ` [PATCH 3/3] Fix subscript completion bugs inside ~[...] Marlon Richert 2023-05-01 22:37 ` Patch 1/3: Fix prefix insertion logic Felipe Contreras 2023-05-01 22:52 ` Bart Schaefer 2023-05-02 4:49 ` Felipe Contreras 2023-05-02 19:16 ` Bart Schaefer 2023-05-02 20:14 ` Felipe Contreras 2023-05-05 11:51 ` Marlon Richert 2023-05-05 12:38 ` Felipe Contreras 2023-05-05 15:04 ` Mikael Magnusson 2023-05-06 17:23 ` Bart Schaefer 2023-05-01 23:36 ` Vin Shelton
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).