[(I)-foo] subscripting in _git

[(I)-foo] subscripting in _git

[Mikael Magnusson]

I noticed while looking at _git-add that _git does a lot of this type of thing:
if (( words[(I)-n|--dry-run] )); then
that particular example is fine, but further down is this
if [[ -n ${line[(I)-f|--force]} ]]; then
which seems to have two problems, first it uses line instead of words,
so it never matches. Second it uses [[ -n ]] which will always be true
as (I) returns a 0 when it didn't match. There appears to be other
occurences of this pattern in the file, and before I go changing all
of them I wanted to ask if I missed something obvious? It also looks
like _arguments changes $words, as I had to do this to make it work.

@@ -38,6 +38,8 @@
 _git-add () {
   local curcontext=$curcontext state line
   declare -A opt_args
+  local -a owords
+  owords=($words)
@@ -63,7 +65,7 @@ _git-add () {
     (file)
       # TODO: Use __git_ignore_line_inside_arguments.
       declare -a ignored_files_alternatives
-      if [[ -n ${line[(I)-f|--force]} ]]; then
+      if (( ${owords[(I)-f|--force]} )); then
         ignored_files_alternatives=(


-- 
Mikael Magnusson

Re: [(I)-foo] subscripting in _git

[Nikolai Weibull]

On Thu, Jul 21, 2011 at 12:32, Mikael Magnusson <mikachu@gmail.com> wrote:
> I noticed while looking at _git-add that _git does a lot of this type of thing:
> if (( words[(I)-n|--dry-run] )); then
> that particular example is fine, but further down is this
> if [[ -n ${line[(I)-f|--force]} ]]; then
> which seems to have two problems, first it uses line instead of words,
> so it never matches.

If you check out the latest revision, however, this has been fixed.  I
fixed it an hour or so ago.  The correct pattern when checking for an
option processed by _arguments is if [[ -n ${opt_args[(I)-o|--option]}
]]; then …; fi.

There are, based on my checks, no other instances of the incorrect pattern.

Re: [(I)-foo] subscripting in _git

[Mikael Magnusson]

On 21 July 2011 13:21, Nikolai Weibull <now@bitwi.se> wrote:
> On Thu, Jul 21, 2011 at 12:32, Mikael Magnusson <mikachu@gmail.com> wrote:
>> I noticed while looking at _git-add that _git does a lot of this type of thing:
>> if (( words[(I)-n|--dry-run] )); then
>> that particular example is fine, but further down is this
>> if [[ -n ${line[(I)-f|--force]} ]]; then
>> which seems to have two problems, first it uses line instead of words,
>> so it never matches.
>
> If you check out the latest revision, however, this has been fixed.  I
> fixed it an hour or so ago.  The correct pattern when checking for an
> option processed by _arguments is if [[ -n ${opt_args[(I)-o|--option]}
> ]]; then …; fi.
>
> There are, based on my checks, no other instances of the incorrect pattern.

You left out this part of my mail:
> Second it uses [[ -n ]] which will always be true as (I) returns a
> 0 when it didn't match.

The obvious part that I was missing is that opt_args is an associative
array, which changes the meaning of (I). :)

-- 
Mikael Magnusson