PATCH 1/5: _git: various fixes

PATCH 1/5: _git: various fixes

[Mikael Magnusson]

Add = to git checkout --conflict= completion
fix transposed [-
git push remote argument is not optional
can use shorter syntax for a check
---
 Completion/Unix/Command/_git | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 5b78a2b..4357b74 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -132,7 +132,7 @@ _git-archive () {
 
   declare -a backend_args
 
-  if (( words[(I)--format=*] > 0 && words[(I)--format=*] < CURRENT )); then
+  if (( words[(b:CURRENT-1:I)--format=*] )); then
     case ${words[$words[(I)--format=*]]#--format=} in
       (zip)
         backend_args=(
@@ -442,7 +442,7 @@ _git-checkout () {
     '(-b -B -t --track --patch --detach)--orphan[create a new orphan branch based at given commit]: :__git_branch_names' \
     '--ignore-skip-worktree-bits[ignores patterns and adds back any files in <paths>]' \
     '(-q --quiet -f --force -m --merge --conflict --patch)'{-m,--merge}'[3way merge current branch, working tree and new branch]' \
-    '(-q --quiet -f --force -m --merge --patch)--conflict[same as --merge, using given merge style]:style:(merge diff3)' \
+    '(-q --quiet -f --force -m --merge --patch)--conflict=[same as --merge, using given merge style]:style:(merge diff3)' \
     '(-)'{-p,--patch}'[interactively select hunks in diff between given tree-ish and working tree]' \
     '(-)--[start file arguments]' \
     '*:: :->branch-or-tree-ish-or-file' && ret=0
@@ -1285,7 +1285,7 @@ _git-push () {
     '(--verify)--no-verify[bybass the pre-push hook]' \
     '--recurse-submodules=[submodule handling]:submodule handling:((check\:"refuse pushing of supermodule if submodule commit cannot be found on the remote"
                                                                     on-demand\:"push all changed submodules"))' \
-    ':: :__git_any_repositories' \
+    ': :__git_any_repositories' \
     '*: :__git_ref_specs' && ret=0
 
   case $state in
@@ -6356,8 +6356,8 @@ __git_setup_revision_options () {
     '*--not[reverses meaning of ^ prefix for revisions that follow]'
     '--all[show all commits from refs]'
     '--branches=-[show all commits from refs/heads]::pattern'
-    '--tags=[-show all commits from refs/tags]::pattern'
-    '--remotes=[-show all commits from refs/remotes]::pattern'
+    '--tags=-[show all commits from refs/tags]::pattern'
+    '--remotes=-[show all commits from refs/remotes]::pattern'
     '--glob=[show all commits from refs matching glob]:pattern'
     '--exclude=[do not include refs matching glob]:pattern'
     '--exclude=[do not include refs matching glob]:pattern'
-- 
2.4.0

PATCH 2/5: _wget: complete headers for --header and add --no-use-server-timestamps

[Mikael Magnusson]

---
 Completion/Unix/Command/_wget | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/Completion/Unix/Command/_wget b/Completion/Unix/Command/_wget
index b8ca2fd..b6feab5 100644
--- a/Completion/Unix/Command/_wget
+++ b/Completion/Unix/Command/_wget
@@ -63,7 +63,7 @@ _arguments -C -s \
   '--default-page=[specify default page name, normally index.html]' \
   '(--adjust-extension -E)'{--adjust-extension,-E}'[save all HTML/CSS documents with proper extensions]' \
   "--ignore-length[ignore \`Content-Length' header field]" \
-  '*--header=:string' \
+  '*--header=[send a custom HTTP header]:header:->header' \
   '--max-redirect=:number' \
   '--proxy-user=:user' \
   '--proxy-password=:password' \
@@ -132,6 +132,7 @@ _arguments -C -s \
   '--no-clobber' \
   '--no-directories' \
   '--no-host-directories' \
+  '--no-use-server-timestamps[do not set timestamp to server provided value]' \
   '--htmlify=:htmlify:' \
   '--no:no:->noflags' \
   '*:URL:_urls' && return 0
@@ -156,4 +157,42 @@ case "$state" in
       '(unix)windows' \
       '(unix windows)nocontrol'
   ;;
+  header)
+    local -a headers
+    headers=(
+             Accept{,-{Charset,Encoding,Language,Datetime}}
+             Authorization
+             Cache-Control
+             Connection
+             Cookie
+             Content-{Length,MD5,Type}
+             Date
+             Expect
+             From
+             Host
+             If-Match
+             If-Modified-Since
+             If-None-Match
+             If-Range
+             If-Unmodified-Since
+             Max-Forwards
+             Pragma
+             Proxy-Authorization
+             Range
+             Referer
+             TE
+             Upgrade
+             User-Agent
+             Via
+             Warning
+             X-Requested-With
+             X-Do-Not-Track
+             DNT
+             X-Forwarded-For
+             X-ATT-DeviceId
+             X-Wap-Profile
+             )
+    headers=($^headers\\:\ )
+    _describe -t header 'HTTP header' headers
+  ;;
 esac
-- 
2.4.0

PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

---
 Completion/Unix/Command/_imagemagick | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Completion/Unix/Command/_imagemagick b/Completion/Unix/Command/_imagemagick
index 115cb01..c43086c 100644
--- a/Completion/Unix/Command/_imagemagick
+++ b/Completion/Unix/Command/_imagemagick
@@ -14,7 +14,7 @@ typeset -A opt_args
 formats=jpg:jpeg:jp2:j2k:jpc:jpx:jpf:tiff:miff:ras:bmp:cgm:dcx:ps:eps:fig:fits:fpx:gif:mpeg:pbm:pgm:ppm:pcd:pcl:pdf:pcx:png:rad:rgb:rgba:rle:sgi:html:shtml:tga:ttf:uil:xcf:xwd:xbm:xpm:yuv
 
 if (( $# )); then
-  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
+  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"
   return
 fi
 
-- 
2.4.0

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Oliver Kiddle]

Mikael Magnusson wrote:
> -  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
> +  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"

_files already does this internally, configurable via the file-patterns
style. From zsh -f or in my full setup, this already works for me. This
same change might equally be applicable to every use of _files with -g
that we have.

You might want to instead check your file-patterns styles to see what
you've configured that broke this.

Oliver

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 7:20 PM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Mikael Magnusson wrote:
>> -  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
>> +  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"
>
> _files already does this internally, configurable via the file-patterns
> style. From zsh -f or in my full setup, this already works for me. This
> same change might equally be applicable to every use of _files with -g
> that we have.
>
> You might want to instead check your file-patterns styles to see what
> you've configured that broke this.

You're right that it works in zsh -f, however, I don't have any
file-patterns styles set and it still doesn't work in my setup.

This is the line that breaks the desired behaviour,
zstyle ':completion:*' list-dirs-first true

Which I find to be somewhat unexpected.

This seems to fix it, and brings the l-d-f case in line with the
default case below it.

diff --git i/Completion/Unix/Command/_imagemagick
w/Completion/Unix/Command/_imagemagick
index c43086c..115cb01 100644
--- i/Completion/Unix/Command/_imagemagick
+++ w/Completion/Unix/Command/_imagemagick
@@ -14,7 +14,7 @@ typeset -A opt_args
 formats=jpg:jpeg:jp2:j2k:jpc:jpx:jpf:tiff:miff:ras:bmp:cgm:dcx:ps:eps:fig:fits:fpx:gif:mpeg:p

 if (( $# )); then
-  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"
+  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
   return
 fi

diff --git i/Completion/Unix/Type/_files w/Completion/Unix/Type/_files
index f4a8fc2..13f4871 100644
--- i/Completion/Unix/Type/_files
+++ w/Completion/Unix/Type/_files
@@ -72,7 +72,7 @@ elif zstyle -t ":completion:${curcontext}:"
list-dirs-first; then
       glob="$glob(#q^-/)"
     fi

-    pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" )
+    pats=( " *(-/):directories:directories
${glob//:/\\:}:globbed-files" '*:all-files' )
   elif [[ "$type" = */* ]] then
     pats=( '*(-/):directories ' '*:all-files ' )
   else

[extra context for reference]
    pats=( '*(-/):directories:directories *(^-/):other-files ' )
  fi
else
  if [[ "$type" = *g* ]]; then

  # People prefer to have directories shown on first try as default.
  # Even if the calling function didn't use -/.
  #
  # if [[ "$type" = */* ]]; then

    pats=( " ${glob//:/\\:}:globbed-files *(-/):directories" '*:all-files '

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 12,  8:12pm, Mikael Magnusson wrote:
} Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
}
} > You might want to instead check your file-patterns styles to see what
} > you've configured that broke this.
} 
} You're right that it works in zsh -f, however, I don't have any
} file-patterns styles set and it still doesn't work in my setup.
} 
} This is the line that breaks the desired behaviour,
} zstyle ':completion:*' list-dirs-first true
} 
} Which I find to be somewhat unexpected.

I'd swear a similar conversation went by on the list sometime in the
past, but I can't find it now.  

} This seems to fix it, and brings the l-d-f case in line with the
} default case below it.

I don't know that I feel strongly about this one way or another, but
according to http://www.zsh.org/mla/workers/2008/msg01119.html you are
not supposed to get an all-files tag when using list-dirs-first.
Instead you're always supposed to get "directories" and "other-files".

So instead of

pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*:all-files' )

(which BTW had a stray line break in the patch you sent) it should be
at most

pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*(-^/):other-files' )

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 8:59 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 12,  8:12pm, Mikael Magnusson wrote:
> } Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
> }
> } > You might want to instead check your file-patterns styles to see what
> } > you've configured that broke this.
> }
> } You're right that it works in zsh -f, however, I don't have any
> } file-patterns styles set and it still doesn't work in my setup.
> }
> } This is the line that breaks the desired behaviour,
> } zstyle ':completion:*' list-dirs-first true
> }
> } Which I find to be somewhat unexpected.
>
> I'd swear a similar conversation went by on the list sometime in the
> past, but I can't find it now.
>
> } This seems to fix it, and brings the l-d-f case in line with the
> } default case below it.
>
> I don't know that I feel strongly about this one way or another, but
> according to http://www.zsh.org/mla/workers/2008/msg01119.html you are
> not supposed to get an all-files tag when using list-dirs-first.
> Instead you're always supposed to get "directories" and "other-files".
>
> So instead of
>
> pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*:all-files' )
>
> (which BTW had a stray line break in the patch you sent) it should be
> at most
>
> pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*(-^/):other-files' )

Okay, it was a very tentative patch so I just pasted it in the gmail
web interface, it usually breaks them. The line you suggested seems to
work fine for me as well. However, the duplicates from the * seem to
get filtered out by somewhere already because my listing is identical
except from the different tag for the files. I suppose if I removed
the directories tag, they would show up under the all-files tag
instead though, so this is indeed more correct.

Is the leading space in that pattern significant in any way, by the way?

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 12,  9:35pm, Mikael Magnusson wrote:
}
} > pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*(-^/):other-files' )
} 
} Is the leading space in that pattern significant in any way, by the way?

I can't think of any reason why it would be.

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 8:59 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 12,  8:12pm, Mikael Magnusson wrote:
> } Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
> }
> } > You might want to instead check your file-patterns styles to see what
> } > you've configured that broke this.
> }
> } You're right that it works in zsh -f, however, I don't have any
> } file-patterns styles set and it still doesn't work in my setup.
> }
> } This is the line that breaks the desired behaviour,
> } zstyle ':completion:*' list-dirs-first true
> }
> } Which I find to be somewhat unexpected.
>
> I'd swear a similar conversation went by on the list sometime in the
> past, but I can't find it now.
>
> } This seems to fix it, and brings the l-d-f case in line with the
> } default case below it.
>
> I don't know that I feel strongly about this one way or another, but
> according to http://www.zsh.org/mla/workers/2008/msg01119.html you are
> not supposed to get an all-files tag when using list-dirs-first.
> Instead you're always supposed to get "directories" and "other-files".
>
> So instead of
>
> pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*:all-files' )
>
> (which BTW had a stray line break in the patch you sent) it should be
> at most
>
> pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*(-^/):other-files' )

I just went to send a new patch with this change, and noticed that the
existing else case for */* does this (under list-dirs-first),

  elif [[ "$type" = */* ]] then
    pats=( '*(-/):directories ' '*:all-files ' )

should this not be other-files too? (It has been all-files since the
original 2008 patch).

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 12, 10:05pm, Mikael Magnusson wrote:
} Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
}
} > pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*(-^/):other-files' )
} 
} I just went to send a new patch with this change, and noticed that the
} existing else case for */* does this (under list-dirs-first),
} 
}   elif [[ "$type" = */* ]] then
}     pats=( '*(-/):directories ' '*:all-files ' )
} 
} should this not be other-files too?

I've lost track of the significance of "$type" here.  It's derived from
the argument to the -g option.  Perhaps list-dirs-first is only meant
to apply to local directories, not to those found somewhere down the
tree?  It's not documented that way, though.

I think we'd have to try it and see whether it makes any difference.

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 10:57 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 12, 10:05pm, Mikael Magnusson wrote:
> } Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
> }
> } > pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" '*(-^/):other-files' )
> }
> } I just went to send a new patch with this change, and noticed that the
> } existing else case for */* does this (under list-dirs-first),
> }
> }   elif [[ "$type" = */* ]] then
> }     pats=( '*(-/):directories ' '*:all-files ' )
> }
> } should this not be other-files too?
>
> I've lost track of the significance of "$type" here.  It's derived from
> the argument to the -g option.  Perhaps list-dirs-first is only meant
> to apply to local directories, not to those found somewhere down the
> tree?  It's not documented that way, though.
>
> I think we'd have to try it and see whether it makes any difference.

Well, I made the change locally and the tag in completions of files to
rmdir changed from all-files to other-files.

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 12, 11:15pm, Mikael Magnusson wrote:
}
} Well, I made the change locally and the tag in completions of files to
} rmdir changed from all-files to other-files.

Sure, but the question is how likely is it that someone has a style
that relies on that tag name?

Also you can force it back to all-files with the file-patterns style,
so there'd have to be a case where the user has list-dirs-first, does
not have file-patterns, and looks for all-files as a tag context.

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 11:44 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 12, 11:15pm, Mikael Magnusson wrote:
> }
> } Well, I made the change locally and the tag in completions of files to
> } rmdir changed from all-files to other-files.
>
> Sure, but the question is how likely is it that someone has a style
> that relies on that tag name?
>
> Also you can force it back to all-files with the file-patterns style,
> so there'd have to be a case where the user has list-dirs-first, does
> not have file-patterns, and looks for all-files as a tag context.

So I guess the uncontroversial thing to do is to just add the
other-files in the fallback case, and leave the middle one alone as
all-files then? I don't see why "my" fallback should be other-files
while the existing fallback is all-files, but I don't care either way,
I just want the files to be completed at all :).

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 13, 12:34am, Mikael Magnusson wrote:
}
} So I guess the uncontroversial thing to do is to just add the
} other-files in the fallback case, and leave the middle one alone as
} all-files then?

If you're trying to be uncontroversial, you should put _files back just
as it was and add a file-patterns style to your own configuration. :-)

} I don't see why "my" fallback should be other-files
} while the existing fallback is all-files, but I don't care either way,
} I just want the files to be completed at all :).

I was implying that you probably CAN change both to other-files but the
opinionated one here seems to be Oliver so let's give him a chance to
chime in.

Anybody else want to comment on the likelyhood of the tag changing from
all-files to other-files causing a problem?

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Thu, Aug 13, 2015 at 12:46 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 13, 12:34am, Mikael Magnusson wrote:
> }
> } So I guess the uncontroversial thing to do is to just add the
> } other-files in the fallback case, and leave the middle one alone as
> } all-files then?
>
> If you're trying to be uncontroversial, you should put _files back just
> as it was and add a file-patterns style to your own configuration. :-)

I don't see what file-patterns style I could add, to complete a
pattern given in a completer and still fall back to other files
generically, in my .zshrc.

> } I don't see why "my" fallback should be other-files
> } while the existing fallback is all-files, but I don't care either way,
> } I just want the files to be completed at all :).
>
> I was implying that you probably CAN change both to other-files but the
> opinionated one here seems to be Oliver so let's give him a chance to
> chime in.

Alright.

> Anybody else want to comment on the likelyhood of the tag changing from
> all-files to other-files causing a problem?

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Oliver Kiddle]

Bart wrote:
> 
> I was implying that you probably CAN change both to other-files but the
> opinionated one here seems to be Oliver so let's give him a chance to
> chime in.

Had I been paying enough attention in 2008 to be opinionated then, I
would have questioned why we need a special style corresponding to a
specific user preference that ought to be covered by file-patterns.

That mail, briefly mentions that the reason for it was that
file-patterns was broken for zmodload completion. _zmodload does:
  _files -W module_path -/g '*.(dll|s[ol]|bundle)(:r)' && ret=0

The use of (:r) there makes this a borderline case where _path_files
should perhaps have been used but it is actually -/g that is causing
the problem.

-/g with compctl gives you directories mixed with globbed files which
was very useful at the time. It is still just about relevant to
_path_files but with _files, if it actually worked, it would just be
overriding user preferences which is missing the whole point of _files.
_files does:
  [[ "$type" = */* ]] && glob="$glob,*(-/)"

Either comma was at one time special to _path_files or this was never
tested. In the patch below, I just remove it.

The one other use of -/g that we have was in _command_names with
_path_files. In that case, I can't see a reason not to use _files
instead.

> Anybody else want to comment on the likelyhood of the tag changing from
> all-files to other-files causing a problem?

I would actually be inclined to suggest that other-files should be
globbed-files: _files is defining three defaults for file-patterns
corresponding to whether -g, -/ or neither was specified. So how do you
override just one of those file patterns? If we define _files without a
-g as being equivalent to _files -g '*(-.)' then a single file-patterns
default would suffice. It'd also mean all-files would only be used as a
fallback and globbed-files would be the tag for all the files when no
glob is specified. It might be better named in that case but it is the
tag we already have for specifically requested files and in that case,
all files are what have been selected. Furthermore, there are issues to
consider of when we actually want a specific set of directories to be
globbed for. The single default I have in mind would be something like:

'%p(-.):globbed-files %p(-/):globbed-dirs *(-/):directories' '*:all-files'

That would be more likely to cause problems for someone's setup than renaming
an all-files or other-files tag however. It would probably be a good
thing if we can avoid every little completion function needing to do
stuff like _files -g '*.png(-.)". -/ would mean we actually want to
select directories. Any opinions?

Patch below is just for the zmodload etc issues mentioned at the
beginning.

Oliver

diff --git a/Completion/Unix/Type/_files b/Completion/Unix/Type/_files
index a8ba9b3..b9ff12b 100644
--- a/Completion/Unix/Type/_files
+++ b/Completion/Unix/Type/_files
@@ -51,7 +51,6 @@ else
 fi
 
 if zstyle -a ":completion:${curcontext}:" file-patterns tmp; then
-  [[ "$type" = */* ]] && glob="$glob,*(-/)"
   pats=()
 
   for i in ${tmp//\%p/${${glob:-\*}//:/\\:}}; do
diff --git a/Completion/Zsh/Command/_zmodload b/Completion/Zsh/Command/_zmodload
index e144b98..57fb990 100644
--- a/Completion/Zsh/Command/_zmodload
+++ b/Completion/Zsh/Command/_zmodload
@@ -68,7 +68,7 @@ else
     _requested loadedmodules expl 'loaded modules' \
       compadd -k 'modules[(R)loaded]' && ret=0
     _requested files expl 'module file' \
-      _files -W module_path -/g '*.(dll|s[ol]|bundle)(:r)' && ret=0
+      _files -W module_path -g '*.(dll|s[ol]|bundle)(:r)' && ret=0
     _requested aliases expl 'module alias' \
       compadd "$suf[@]" -k 'modules[(R)alias*]' && ret=0
   done
diff --git a/Completion/Zsh/Type/_command_names b/Completion/Zsh/Type/_command_names
index d9fc62d..940f341 100644
--- a/Completion/Zsh/Type/_command_names
+++ b/Completion/Zsh/Type/_command_names
@@ -17,9 +17,7 @@ defs=(
 )
 
 [[ -n "$path[(r).]" || $PREFIX = */* ]] &&
-  defs=( "$defs[@]"
-         'executables:executable file or directory:_path_files -/g \*\(-\*\)'
-  )
+    defs+=( 'executables:executable file:_files -g \*\(-\*\)' )
 
 if [[ "$1" = -e ]]; then
   shift

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 13, 12:12pm, Oliver Kiddle wrote:
}
} Had I been paying enough attention in 2008 to be opinionated then, I
} would have questioned why we need a special style corresponding to a
} specific user preference that ought to be covered by file-patterns.

As I think I've mentioned elsewhere, file-patterns seems to take
precedence over list-dirst-first anyway.  So I suppose it could be
interpreted as a shorthand, except for the other-files business.

} _files does:
}   [[ "$type" = */* ]] && glob="$glob,*(-/)"
} 
} Either comma was at one time special to _path_files or this was never
} tested. In the patch below, I just remove it.

That bit is probably trying to do the trick of merging together glob
qualifiers, and just gets it wrong.  I.e. here:

    # add `^-/' after `#q' glob qualifier if not there already
    if [[ "$glob" = (#b)(*\(\#q)(*\)) ]]; then
      [[ $match[2] != \^-/* ]] &&
      glob="${match[1]}^-/,${match[2]}"
    else
      glob="$glob(#q^-/)"
    fi

} I would actually be inclined to suggest that other-files should be
} globbed-files: _files is defining three defaults for file-patterns
} corresponding to whether -g, -/ or neither was specified. So how do you
} override just one of those file patterns? If we define _files without a
} -g as being equivalent to _files -g '*(-.)' then a single file-patterns
} default would suffice. It'd also mean all-files would only be used as a
} fallback and globbed-files would be the tag for all the files when no
} glob is specified.

Does it ever make sense to use  _files -g '*(/)' ?  Or should -/ always
be used in that case?

} It would probably be a good
} thing if we can avoid every little completion function needing to do
} stuff like _files -g '*.png(-.)". -/ would mean we actually want to
} select directories. Any opinions?

I'm not sure I understand that part of the issue, yet.

-- 
Barton E. Schaefer

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Oliver Kiddle]

Bart wrote:
> As I think I've mentioned elsewhere, file-patterns seems to take
> precedence over list-dirst-first anyway.  So I suppose it could be
> interpreted as a shorthand, except for the other-files business.

Yes, it is harmless enough as a shorthand.

> That bit is probably trying to do the trick of merging together glob
> qualifiers, and just gets it wrong.  I.e. here:
> 
>     # add `^-/' after `#q' glob qualifier if not there already
>     if [[ "$glob" = (#b)(*\(\#q)(*\)) ]]; then
>       [[ $match[2] != \^-/* ]] &&
>       glob="${match[1]}^-/,${match[2]}"
>     else
>       glob="$glob(#q^-/)"
>     fi

That bit is also getting it wrong: , in a list of glob qualifiers is an
OR and it wants an AND in this case. Just glob="$glob(#q^-/)" would
have done the job.
(The older attempt at merging qualifiers did want an OR which is harder
to do).

> Does it ever make sense to use  _files -g '*(/)' ?  Or should -/ always
> be used in that case?

In that exact example, -/ should be used but there's definitely
use-cases for globs matching only some directories. For example, to
select a git repository you might want *(e:[[ -d \$REPLY/.git ]]:)

We don't tend to bother in the functions because the directories get
picked up for the directories tag anyway and there's no way to tell the
selected directories apart from those only there because they might be
part of the path to the final directory. You can't use complist colours
because that works on the basis of the group rather than the tag. If you
use separate groups then you get matches duplicated.

Instead of *(-/) for the directories, we ideally need to negate the glob
specified with -g. The trouble is that it is actually quite hard to get
the opposite of a glob. The opposite of *.ext(ab,cd) is something like
^*.ext *.ext(^a,^b)(^c,^d)
It'd probably be easier to expand the glob in an array and use that
array as a filter. I'd be interested if anyone has any ideas on how to
do this.

The patch below strips the defaults down to a single pattern regardless
of -/ and -g. This makes things more consistent with the use of
a file-patterns style.

Oliver

diff --git a/Completion/Unix/Type/_files b/Completion/Unix/Type/_files
index a8ba9b3..fe0780a 100644
--- a/Completion/Unix/Type/_files
+++ b/Completion/Unix/Type/_files
@@ -33,6 +33,8 @@ if (( $tmp[(I)-g*] )); then
   # add `#q' to the beginning of any glob qualifier if not there already
   [[ "$glob" = (#b)(*\()([^\|\~]##\)) && $match[2] != \#q* ]] &&
       glob="${match[1]}#q${match[2]}"
+elif [[ $type = */* ]]; then
+  glob="*(-/)"
 fi
 tmp=$opts[(I)-F]
 if (( tmp )); then
@@ -51,59 +53,21 @@ else
 fi
 
 if zstyle -a ":completion:${curcontext}:" file-patterns tmp; then
-  [[ "$type" = */* ]] && glob="$glob,*(-/)"
   pats=()
 
   for i in ${tmp//\%p/${${glob:-\*}//:/\\:}}; do
     if [[ $i = *[^\\]:* ]]; then
-      pats=( "$pats[@]" " $i " )
+      pats+=( " $i " )
     else
-      pats=( "$pats[@]" " ${i}:files " )
+      pats+=( " ${i}:files " )
     fi
   done
 elif zstyle -t ":completion:${curcontext}:" list-dirs-first; then
-  if [[ "$type" = *g* ]]; then
-
-    # add `^-/' after `#q' glob qualifier if not there already
-    if [[ "$glob" = (#b)(*\(\#q)(*\)) ]]; then
-      [[ $match[2] != \^-/* ]] &&
-      glob="${match[1]}^-/,${match[2]}"
-    else
-      glob="$glob(#q^-/)"
-    fi
-
-    pats=( " *(-/):directories:directories ${glob//:/\\:}:globbed-files" )
-  elif [[ "$type" = */* ]] then
-    pats=( '*(-/):directories ' '*:all-files ' )
-  else
-    pats=( '*(-/):directories:directories *(^-/):other-files ' )
-  fi
+  pats=( " *(-/):directories:directory ${${glob:-*}//:/\\:}(#q^-/):globbed-files" '*:all-files' )
 else
-  if [[ "$type" = *g* ]]; then
-
   # People prefer to have directories shown on first try as default.
   # Even if the calling function didn't use -/.
-  #
-  # if [[ "$type" = */* ]]; then
-
-    pats=( " ${glob//:/\\:}:globbed-files *(-/):directories" '*:all-files '
-
-    ### We could allow _next_tags to offer only globbed-files or directories
-    ### by adding:
-    ###   " ${glob//:/\\:}:only-globbed-files " ' *(-/):only-directories '
-
-      )
-
-  # else
-  #   pats=( " ${glob//:/\\:}:globbed-files "
-  #          '*(-/):directories ' '*:all-files ' )
-  # fi
-
-  elif [[ "$type" = */* ]]; then
-    pats=( '*(-/):directories ' '*:all-files ' )
-  else
-    pats=( '*:all-files ' )
-  fi
+  pats=( "${${glob:-*}//:/\\:}:globbed-files *(-/):directories" '*:all-files ' )
 fi
 
 tried=()

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Fri, Aug 14, 2015 at 10:44 AM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Bart wrote:
>> As I think I've mentioned elsewhere, file-patterns seems to take
>> precedence over list-dirst-first anyway.  So I suppose it could be
>> interpreted as a shorthand, except for the other-files business.
>
> Yes, it is harmless enough as a shorthand.
[...]
>> Does it ever make sense to use  _files -g '*(/)' ?  Or should -/ always
>> be used in that case?
>
> In that exact example, -/ should be used but there's definitely
> use-cases for globs matching only some directories. For example, to
> select a git repository you might want *(e:[[ -d \$REPLY/.git ]]:)

I think Bart just meant (/) vs (-/), but I'm not sure.

> We don't tend to bother in the functions because the directories get
> picked up for the directories tag anyway and there's no way to tell the
> selected directories apart from those only there because they might be
> part of the path to the final directory. You can't use complist colours
> because that works on the basis of the group rather than the tag. If you
> use separate groups then you get matches duplicated.
>
> Instead of *(-/) for the directories, we ideally need to negate the glob
> specified with -g. The trouble is that it is actually quite hard to get
> the opposite of a glob. The opposite of *.ext(ab,cd) is something like
> ^*.ext *.ext(^a,^b)(^c,^d)
> It'd probably be easier to expand the glob in an array and use that
> array as a filter. I'd be interested if anyone has any ideas on how to
> do this.

Something like this?
all=(*) positive=(*.ext(ab,cd))
negative=( ${all:|positive} )

You can't actually write that in a glob:tag part though... (And doing
it in (e:...:) would be very slow)

[...]

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 14,  6:18pm, Mikael Magnusson wrote:
}
} >> Does it ever make sense to use  _files -g '*(/)' ?  Or should -/ always
} >> be used in that case?
} 
} I think Bart just meant (/) vs (-/), but I'm not sure.

Actually I meant  _files -g '*(/)'  vs.  _files -/

I *think* Oliver answered the right question; I was not refering to (-/)
as a glob qualifier at all.

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 5:05 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
> ---
>  Completion/Unix/Command/_imagemagick | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Completion/Unix/Command/_imagemagick b/Completion/Unix/Command/_imagemagick
> index 115cb01..c43086c 100644
> --- a/Completion/Unix/Command/_imagemagick
> +++ b/Completion/Unix/Command/_imagemagick
> @@ -14,7 +14,7 @@ typeset -A opt_args
>  formats=jpg:jpeg:jp2:j2k:jpc:jpx:jpf:tiff:miff:ras:bmp:cgm:dcx:ps:eps:fig:fits:fpx:gif:mpeg:pbm:pgm:ppm:pcd:pcl:pdf:pcx:png:rad:rgb:rgba:rle:sgi:html:shtml:tga:ttf:uil:xcf:xwd:xbm:xpm:yuv
>
>  if (( $# )); then
> -  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
> +  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"
>    return
>  fi

Now that I'm looking at this line for the 500th time, I realize that
the ~ in there makes absolutely no sense, and shouldn't really work.
However, it seems to be totally ignored and harmless. I tried changing
formats to an array, and using this instead,
_files "$@" -g "*.(#i)(${(j:|:)~formats})(-.)"

And it works the same, next I tried (actually I did the other way
around but this is easier to follow)
_files "$@" -g "*.(#i)(${(~j:|:)formats})(-.)"
but this fails because for some reason this ~ is not ignored and the
pattern is evaluated at this point, expands to nothing, because
NULL_GLOB is set in completion, and we don't pass anything as an
argument to -g (because the * is quoted. If there was a literal *.jpg
it would probably do something else).

Long story short, let's do this while we're messing around in this
file, so this semi-anti-pattern doesn't spread when people look at
this file for modeling other completions.

diff --git i/Completion/Unix/Command/_imagemagick
w/Completion/Unix/Command/_imagemagick
index 115cb01..3b0b877 100644
--- i/Completion/Unix/Command/_imagemagick
+++ w/Completion/Unix/Command/_imagemagick
@@ -11,10 +11,10 @@ typeset -A opt_args
 #
 # and certainly many other things...

-formats=jpg:jpeg:jp2:j2k:jpc:jpx:jpf:tiff:miff:ras:bmp:cgm:dcx:ps:eps:fig:fits:fpx:gif:mpeg:pbm:pgm:ppm:pcd:pcl:pdf:pcx:png:rad:rgb:rgba:rle:sgi:html:shtml:tga:ttf:uil:xcf:xwd:xbm:xpm:yuv
+formats=(jpg jpeg jp2 j2k jpc jpx jpf tiff miff ras bmp cgm dcx ps
eps fig fits fpx gif mpeg pbm pgm ppm pcd pcl pdf pcx png rad rgb rgba
rle sgi html shtml tga ttf uil xcf xwd xbm xpm yuv)

 if (( $# )); then
-  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
+  _files "$@" -g "*.(#i)(${(j:|:)formats})(-.)"
   return
 fi

@@ -444,7 +444,7 @@ case "$service" in
       '*-filter:filter type for resizing:(Point Box Triangle Hermite
Hanning Hamming Blackman Gaussian Quadratic Cubic Catrom Mitchell
Lanczos Bessel Sinc)' \
       '*-flip[vertical mirror image]' \
       '*-flop[horizontal mirror image]' \
-      "*-format:output file format:(${formats//:/ })" \
+      "*-format:output file format:($formats)" \
       '*-font:annotation font:_x_font' \
       '*-frame:border dimensions (<width>x<height>+<out>+<in>)' \
       '*-fuzz:maximum distance for equal colors' \

-- 
Mikael Magnusson

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Bart Schaefer]

On Aug 15, 12:31am, Mikael Magnusson wrote:
} Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
}
} > -  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
} > +  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"
} 
} Now that I'm looking at this line for the 500th time, I realize that
} the ~ in there makes absolutely no sense, and shouldn't really work.

The ~ there is in fact useless, because the expansion is quoted, so
converting characters within the elements of $format into patterns
doesn't do anything.

In fact it's more likely that ${(b)formats} is wanted so that pattern
charactes in each of the format strings would be quoted, if there were
any.  (That *will* persist because it's a textual change inserting
backslashes.)  Then they'd remain non-special when the positional
paramter is expanded later by _files.

} However, it seems to be totally ignored and harmless. I tried changing
} formats to an array, and using this instead,
} _files "$@" -g "*.(#i)(${(j:|:)~formats})(-.)"

Same thing.

} And it works the same, next I tried (actually I did the other way
} around but this is easier to follow)
} _files "$@" -g "*.(#i)(${(~j:|:)formats})(-.)"

When you have an array, "${(j:|:)array}" generates the set consisting of
each of the quoted array elements, and then joins them.  The vertical
bar is not part of the quoted regions, but it's also not interpreted as
a pattern character.

"${(~j:|:)array}" generates the quoted elements and then joins them
with the pattern character represented by vertical bar, so you get the
expansion immediately as you noted.

} -formats=jpg:jpeg:jp2:j2k:jpc:jpx:jpf:tiff:miff:ras:bmp:cgm:dcx:ps:eps:fig:fits:fpx:gif:mpeg:pbm:pgm:ppm:pcd:pcl:pdf:pcx:png:rad:rgb:rgba:rle:sgi:html:shtml:tga:ttf:uil:xcf:xwd:xbm:xpm:yuv
} +formats=(jpg jpeg jp2 j2k jpc jpx jpf tiff miff ras bmp cgm dcx ps
} eps fig fits fpx gif mpeg pbm pgm ppm pcd pcl pdf pcx png rad rgb rgba
} rle sgi html shtml tga ttf uil xcf xwd xbm xpm yuv)

If you're going to do that, you should also declare formats as an array
instead of a string, just for completeness.

Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson]

On Sat, Aug 15, 2015 at 2:05 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 15, 12:31am, Mikael Magnusson wrote:
> } Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
> }
> } > -  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
> } > +  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"
> }
> } Now that I'm looking at this line for the 500th time, I realize that
> } the ~ in there makes absolutely no sense, and shouldn't really work.
>
> The ~ there is in fact useless, because the expansion is quoted, so
> converting characters within the elements of $format into patterns
> doesn't do anything.
>
> In fact it's more likely that ${(b)formats} is wanted so that pattern
> charactes in each of the format strings would be quoted, if there were
> any.  (That *will* persist because it's a textual change inserting
> backslashes.)  Then they'd remain non-special when the positional
> paramter is expanded later by _files.
>
> } However, it seems to be totally ignored and harmless. I tried changing
> } formats to an array, and using this instead,
> } _files "$@" -g "*.(#i)(${(j:|:)~formats})(-.)"
>
> Same thing.
>
> } And it works the same, next I tried (actually I did the other way
> } around but this is easier to follow)
> } _files "$@" -g "*.(#i)(${(~j:|:)formats})(-.)"
>
> When you have an array, "${(j:|:)array}" generates the set consisting of
> each of the quoted array elements, and then joins them.  The vertical
> bar is not part of the quoted regions, but it's also not interpreted as
> a pattern character.
>
> "${(~j:|:)array}" generates the quoted elements and then joins them
> with the pattern character represented by vertical bar, so you get the
> expansion immediately as you noted.

Okay, I guess that makes sense on some level. :)

> } -formats=jpg:jpeg:jp2:j2k:jpc:jpx:jpf:tiff:miff:ras:bmp:cgm:dcx:ps:eps:fig:fits:fpx:gif:mpeg:pbm:pgm:ppm:pcd:pcl:pdf:pcx:png:rad:rgb:rgba:rle:sgi:html:shtml:tga:ttf:uil:xcf:xwd:xbm:xpm:yuv
> } +formats=(jpg jpeg jp2 j2k jpc jpx jpf tiff miff ras bmp cgm dcx ps
> } eps fig fits fpx gif mpeg pbm pgm ppm pcd pcl pdf pcx png rad rgb rgba
> } rle sgi html shtml tga ttf uil xcf xwd xbm xpm yuv)
>
> If you're going to do that, you should also declare formats as an array
> instead of a string, just for completeness.

Good point, I'll squash it in when pushing.

-- 
Mikael Magnusson

PATCH 4/5: _sort: Fix syntax error

[Mikael Magnusson]

---
 Completion/Unix/Command/_sort | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Completion/Unix/Command/_sort b/Completion/Unix/Command/_sort
index 2e7f0a0..1ad57f4 100644
--- a/Completion/Unix/Command/_sort
+++ b/Completion/Unix/Command/_sort
@@ -55,7 +55,7 @@ case $variant in
   netbsd*|dragonfly*)
     args+=(
       "${ordering}-l[sort by string length of field]"
-      "(-s)-S[don't use stable sort"
+      "(-s)-S[don't use stable sort]"
     )
   ;|
   openbsd*)
-- 
2.4.0

PATCH 5/5: _strftime: Add completion for zsh/datetime's strftime builtin

[Mikael Magnusson]

---
 Completion/Zsh/Command/_strftime | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 Completion/Zsh/Command/_strftime

diff --git a/Completion/Zsh/Command/_strftime b/Completion/Zsh/Command/_strftime
new file mode 100644
index 0000000..0be7b07
--- /dev/null
+++ b/Completion/Zsh/Command/_strftime
@@ -0,0 +1,12 @@
+#compdef strftime
+
+local ret=1 expl
+
+_arguments -S -A '-*' -s \
+  '-q[run quietly]' \
+  '-r[reverse lookup using strptime]' \
+  '-s+[assign result to parameter]:param:_parameters' \
+  '1:format: _date_formats' \
+  '2:epoch time (or date string with -r)' && ret=0
+
+return ret
-- 
2.4.0