How to do completion of toggle flag sequences separated by +/-

How to do completion of toggle flag sequences separated by +/-

[Haakon Riiser]

 I'm still working on perfecting the FFmpeg completion function, and one 
 of the most important things missing is good completion of toggle flags.

 Here's an example of FFmpeg's toggle flag syntax:

 ffmpeg -flags +gmc-global_header+cgop

 In the above example, three toggle flags are set:

 gmc is enabled (+)
 global_header is disabled (-)
 cgop is enabled (+)

 Ideally, I would like the completion to behave like this:

 ffmpeg -flags ^I

 Typing the above should generate two matches: "+" and "-". Next,

 ffmpeg -flags +^I

 should generate a list of all possible flag values (gmc, cgop, etc). 
 Next, select one of the flags, e.g.:

 ffmpeg -flags +gmc^I

 An important point here is that *no space should be inserted* after the 
 inserted flag; the cursor must be placed immediately after "+gmc". 
 Pressing TAB again should repeat the process by again suggesting "+" or 
 "-" to add more flags. Once I'm done adding flags, I type in a space and 
 completion should no longer be in toggle flag mode.

 Achieving this seems very hard to do, because there could be ambiguity 
 in some cases. Assume that "foo" and "foobar" are both valid flags, and 
 I type this:

 ffmpeg -flags +foo^I

 Here, there are two possibilities: Either suggest "+" or "-" as if a 
 new flag is to be completed (because +foo is a valid flag), or complete 
 to +foobar because there is also a flag by that name. Resolving this is 
 probably complicated, so I'll probably compromise on something. Does 
 anyone have any suggestions on how to get as close to the above ideal 
 completion behavior using standard zsh completion functions (_values, 
 _arguments, etc)?

-- 
  Haakon

Re: How to do completion of toggle flag sequences separated by +/-

[Peter Stephenson]

On Sun, 01 May 2011 12:16:05 +0200
Haakon Riiser <haakon.riiser@fys.uio.no> wrote:
>  I'm still working on perfecting the FFmpeg completion function, and one 
>  of the most important things missing is good completion of toggle flags.
> 
>  Here's an example of FFmpeg's toggle flag syntax:
> 
>  ffmpeg -flags +gmc-global_header+cgop
> 
>  In the above example, three toggle flags are set:
> 
>  gmc is enabled (+)
>  global_header is disabled (-)
>  cgop is enabled (+)

I don't think there's a prepackaged way of doing this.  I've had some
luck with the following.  One thing it doesn't do is treat the + and -
as separate from the options following, but a bit of patience might sort
that out.

It's actually written to complete options for the fictitious "flags"
command, so you also have to insinuate it into ffmpeg completion
somehow.


#compdef flags

# Attempt to get something that completes
# +flag1+flag2-flag3 etc.

_flag_options() {
  local expl
  _wanted options expl 'option' compadd -S '' -- {-,+}${^flag_options}
}

_more_flag_options() {
  compset -p $1 && _flag_options
}

_new_flag_options() {
  compset -P '*' && _flag_options
}

_flags() {
  local -a flag_options
  flag_options=(foo foobar bar)

  local match mbegin mend
  integer ret=1

  if [[ $PREFIX = (#b)(*)[-+]([^-+]#) ]]; then
    if [[ -n ${flag_options[(R)$match[2]]} ]]; then
      _new_flag_options && ret=0
    fi
    if [[ -n ${flag_options[(R)$match[2]?*]} ]]; then
      _more_flag_options ${#match[1]} && ret=0
    fi
  else
    _flag_options && ret=0
  fi

  return $ret
}

_flags "$@"


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: How to do completion of toggle flag sequences separated by +/-

[Haakon Riiser]

 On Sun, 1 May 2011 17:18:46 +0100, Peter Stephenson 
 <p.w.stephenson@ntlworld.com> wrote:
> On Sun, 01 May 2011 12:16:05 +0200
> Haakon Riiser <haakon.riiser@fys.uio.no> wrote:
>>  I'm still working on perfecting the FFmpeg completion function, and 
>> one
>>  of the most important things missing is good completion of toggle 
>> flags.
>>
>>  Here's an example of FFmpeg's toggle flag syntax:
>>
>>  ffmpeg -flags +gmc-global_header+cgop
>> [...]
>
> I don't think there's a prepackaged way of doing this.  I've had some
> luck with the following.  One thing it doesn't do is treat the + and 
> -
> as separate from the options following, but a bit of patience might 
> sort
> that out.
>
> It's actually written to complete options for the fictitious "flags"
> command, so you also have to insinuate it into ffmpeg completion
> somehow.
> [...]

 Thanks, that's an excellent starting point. I will try to integrate 
 this into _ffmpeg shortly.

 By the way: When defining sub-functions inside a completion function, 
 is it recommended to test for their existence before defining them? 
 E.g.,

 (( $+functions[_ffmpeg_foo] )) || _ffmpeg_foo() {
   ...
 }

 I see this in a lot of completers included with zsh, and I assumed it 
 was done for performance reasons (although it seems strange that the 
 performance hit could be significant, considering that the code is only 
 executed once per TAB keypress).

-- 
  Haakon

Re: How to do completion of toggle flag sequences separated by +/-

[Peter Stephenson]

On Mon, 2 May 2011 11:16:06 +0200
Haakon Riiser <haakon.riiser@fys.uio.no> wrote:
>  By the way: When defining sub-functions inside a completion
> function, is it recommended to test for their existence before
> defining them? E.g.,
> 
>  (( $+functions[_ffmpeg_foo] )) || _ffmpeg_foo() {
>    ...
>  }
> 
>  I see this in a lot of completers included with zsh, and I assumed
> it was done for performance reasons (although it seems strange that
> the performance hit could be significant, considering that the code
> is only executed once per TAB keypress).

That's certainly normal, but I think it's less for performance reasons
than to allow you to overrided _ffmpeg_foo (or whatever) in another
file.  Note that the (( ... )) expression is true even if _ffmpeg_foo is
marked for autoload.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK
 



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: How to do completion of toggle flag sequences separated by +/-

[Haakon Riiser]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

 On Tue, 3 May 2011 11:27:42 +0100, Peter Stephenson 
 <Peter.Stephenson@csr.com> wrote:
> On Mon, 2 May 2011 11:16:06 +0200
> Haakon Riiser <haakon.riiser@fys.uio.no> wrote:
>>  By the way: When defining sub-functions inside a completion
>> function, is it recommended to test for their existence before
>> defining them? E.g.,
>>
>>  (( $+functions[_ffmpeg_foo] )) || _ffmpeg_foo() {
>>    ...
>>  }
>>
>>  I see this in a lot of completers included with zsh, and I assumed
>> it was done for performance reasons (although it seems strange that
>> the performance hit could be significant, considering that the code
>> is only executed once per TAB keypress).
>
> That's certainly normal, but I think it's less for performance 
> reasons
> than to allow you to overrided _ffmpeg_foo (or whatever) in another
> file.  Note that the (( ... )) expression is true even if _ffmpeg_foo 
> is
> marked for autoload.

 Ah, I see. Doesn't seem to be a reason not to do this then, so I used 
 the same convention for the other stuff I added.

 Attached to this email you will find a patch that implements toggle 
 flags using the method you posted earlier in this thread (thanks again 
 for that, btw). The ffmpeg completion function is pretty cryptic now, 
 but at least it should be very complete and _very_ future-proof. :)

-- 
  Haakon

[-- Attachment #2: ffmpeg-flags-completion.patch --]
[-- Type: text/x-patch, Size: 5232 bytes --]

Index: _ffmpeg
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_ffmpeg,v
retrieving revision 1.4
diff -u -1 -0 -r1.4 _ffmpeg
--- _ffmpeg	1 May 2011 15:46:47 -0000	1.4
+++ _ffmpeg	3 May 2011 13:41:48 -0000
@@ -36,41 +36,87 @@
     pix_fmts=($(_ffmpeg_list_pix_fmts))
     _wanted ffmpeg-pix-fmts expl 'set pixel format' compadd -a pix_fmts
 }
 
 (( $+functions[_ffmpeg_bsfs] )) || _ffmpeg_bsfs() {
     local bsfs
     bsfs=(${${(f)"$(_call_program bsfs $words[1] -bsfs 2>/dev/null)"}:#*:})
     _wanted ffmpeg-bsfs expl 'set bitstream filter' compadd -a bsfs
 }
 
+typeset -A _ffmpeg_flags
+
+(( $+functions[_ffmpeg_flag_options] )) || _ffmpeg_flag_options() {
+    local expl
+    _wanted options expl 'select flags' compadd -S '' -- {-,+}${^flag_options}
+}
+
+(( $+functions[_ffmpeg_more_flag_options] )) || _ffmpeg_more_flag_options() {
+    compset -p $1 && _ffmpeg_flag_options
+}
+
+(( $+functions[_ffmpeg_new_flag_options] )) || _ffmpeg_new_flag_options() {
+    compset -P '*' && _ffmpeg_flag_options
+}
+
+(( $+functions[_ffmpeg_flags] )) || _ffmpeg_flags() {
+    local -a flag_options
+    eval "flag_options=(\${=_ffmpeg_flags[$1]})"
+
+    local match mbegin mend
+    integer ret=1
+
+    if [[ $PREFIX = (#b)(*)[-+]([^-+]#) ]]; then 
+        if [[ -n ${flag_options[(R)$match[2]]} ]]; then
+            _ffmpeg_new_flag_options && ret=0
+        fi 
+        if [[ -n ${flag_options[(R)$match[2]?*]} ]]; then
+            _ffmpeg_more_flag_options ${#match[1]} && ret=0
+        fi
+    else
+        _ffmpeg_flag_options && ret=0
+    fi
+
+    return $ret
+}
+
+(( $+functions[_ffmpeg_register_lastopt_values] )) || _ffmpeg_register_lastopt_values() {
+    if (( lastopt_takesargs )); then
+        lastopt+=":$lastopt_description:"
+        if (( $#lastopt_values )); then
+            if [[ $lastopt_type == flags ]]; then
+                flagtype=${${lastopt%%:*}#-}
+                lastopt+="->$flagtype"
+                _ffmpeg_flags[$flagtype]="${lastopt_values[*]}"
+            else
+                lastopt+="(${lastopt_values[*]})"
+            fi
+        fi
+    fi
+    _ffmpeg_argspecs+=$lastopt
+}
+
 local -a _ffmpeg_argspecs
 {
     local lastopt
     local lastopt_description
     local lastopt_takesargs
+    local lastopt_type
     local -a lastopt_values
 
     _call_program options $words[1] -h 2>/dev/null | while IFS=$'\n' read -r; do
         if [[ $REPLY == -* ]]; then
-            if [[ -n $lastopt ]]; then
-                if (( lastopt_takesargs )); then
-                    lastopt+=":$lastopt_description:"
-                    if (( $#lastopt_values )); then
-                        lastopt+="(${lastopt_values[*]})"
-                    fi
-                fi
-                _ffmpeg_argspecs+=$lastopt
-            fi
+            [[ -n $lastopt ]] && _ffmpeg_register_lastopt_values
             lastopt=${REPLY%%[[:space:]]*}
             lastopt_description=${REPLY##-[^[:space:]]##[[:space:]]##}
-            if [[ $lastopt_description == '<'* ]]; then
+            if [[ $lastopt_description == (#b)'<'(?##)'>'* ]]; then
+                lastopt_type=$match[1]
                 lastopt_description=${lastopt_description##<[^[:space:]]##>[[:space:]]##[^[:space:]]##[[:space:]]#}
                 if [[ -z $lastopt_description ]]; then
                     lastopt_description=$lastopt
                 fi
                 lastopt_description=${lastopt_description//:/\\:}
             elif [[ $lastopt_description == [^[:space:]]##[[:space:]][[:space:]]* ]]; then
                 local example=${lastopt_description%% *}
                 example=${example//:/\\:}
                 lastopt_description=${lastopt_description##[^[:space:]]##[[:space:]]##}
                 lastopt_description=${lastopt_description//:/\\:}
@@ -106,29 +152,21 @@
                 fi
             fi
             lastopt_values=()
         elif [[ $REPLY == ' '* ]]; then
             REPLY=${REPLY##[[:space:]]##}
             REPLY=${REPLY%%[[:space:]]##*}
             lastopt_takesargs=1
             lastopt_values+=$REPLY
         fi
     done
-    if [[ -n $lastopt ]]; then
-        if (( lastopt_takesargs )); then
-            lastopt+=":$lastopt_description:"
-            if (( $#lastopt_values )); then
-                lastopt+="(${lastopt_values[*]})"
-            fi
-        fi
-        _ffmpeg_argspecs+=$lastopt
-    fi
+    [[ -n $lastopt ]] && _ffmpeg_register_lastopt_values
 }
 
 _arguments -S \
     "${_ffmpeg_argspecs[@]}" \
     '*:output file:_files' \
     && return 0
 
 [[ "$state" == "vfilters" ]] &&
     _values -s , -S = 'video filters' \
     'aspect:set aspect ratio (rational number X\:Y or decimal number):' \
@@ -146,11 +184,14 @@
     'nullsrc' \
     'nullsink' \
     && return 0
 
 [[ "$state" == "format" ]] &&
     _values -s : -S = 'convert input video to one of the specified pixel formats' $(_ffmpeg_list_pix_fmts) && return 0
 
 [[ "$state" == "noformat" ]] &&
     _values -s : -S = 'disable specified pixel formats by force' $(_ffmpeg_list_pix_fmts) && return 0
 
+[[ -n $state && -n $_ffmpeg_flags[$state] ]] &&
+    _ffmpeg_flags $state && return 0
+
 return 1