zsh-workers
 help / color / mirror / code / Atom feed
* 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

* 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: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

* 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

* [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

* [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

* [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-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 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

* 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

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).