Fabien COELHO wrote:
>
> Please find attached a patch which helps to avoid typos in property names.
I like the idea.
> MESSAGE
>
> Improve svn command bash auto-completion wrt property subcommands
> - property names are completed
> - revision properties are suggested --revprop and --revision ...
> - possible values are suggested for some properties
> - user-defined file and revision properties can be added
> with environment variables SVN_FILE_PROPS and SVN_REV_PROPS
Please read the guidelines for log messages at
<file:///home/julianfoad/src/subversion/www/hacking.html#log-messages>.
> Index: tools/client-side/bash_completion
> ===================================================================
> --- tools/client-side/bash_completion (revision 17245)
> +++ tools/client-side/bash_completion (working copy)
> @@ -10,12 +10,17 @@
>
> shopt -s extglob
>
> +# env SVN_FILE_PROPS: other properties on files
> +# env SVN_REV_PROPS: other properties on revisions
> _svn()
> {
> local cur cmds cmdOpts pOpts mOpts rOpts qOpts nOpts optsParam opt
> local helpCmds optBase i
>
> COMPREPLY=()
> + # drop ":" separator to handle svn:* property names properly?
"?" ? I assume you're asking reviewers whether this is a good idea. I don't
know the answer.
> + # this seems to break file completion within file:// urls
That's a pity.
> + COMP_WORDBREAKS=${COMP_WORDBREAKS/:/}
> cur=${COMP_WORDS[COMP_CWORD]}
> + # some special partial help about --revision...
> + if [[ ${COMP_WORDS[COMP_CWORD-1]} = '--revision' ]] ; then
> + COMPREPLY=( $( compgen -W "HEAD BASE PREV COMMITTED 0 {" -- $cur ) )
> + return 0
> + fi
> +
> + # what about --author and others?
> +
> + # file: and other url completion?
> + #if [[ $cur == "file:*" ]] ; then
> + # COMP_WORDBREAKS="$COMP_WORDBREAKS:"
> + # return 0
> + #fi
Please try not to leave half-baked ideas in a patch submission. Either take
out bits like this, or say "I'd like comments on the ideas in this patch; it's
not ready for committing."
> @@ -38,8 +58,97 @@
> |--diff-cmd|--diff3-cmd|--editor-cmd|--old|--new|
> |--config-dir|--native-eol|--limit"
>
> - # if not typing an option, or if the previous option required a
> - # parameter, then fallback on ordinary filename expansion
> + # svn:* and other (env SVN_FILE_PROPS and SVN_REV_PROPS) properties
> + local svnProps revProps allProps psCmds propCmds
> +
> + # svn file properties
> + svnProps="svn:keywords svn:executable svn:needs-lock svn:externals"
> + svnProps="svn:ignore svn:eol-style svn:mime-type $svnProps"
> + [ "$SVN_FILE_PROPS" ] && svnProps="$svnProps $SVN_FILE_PROPS"
> +
> + # svn revision properties
> + revProps="svn:author svn:log svn:date"
> + [ "$SVN_REV_PROPS" ] && revProps="$revProps $SVN_REV_PROPS"
> +
> + # all properties
> + allProps="$svnProps $revProps"
> +
> + # subcommands that expect property names
> + psCmds='propset|pset|ps'
> + propCmds="$psCmds|propget|pget|pg|propedit|pedit|pe|propdel|pdel|pd"
> +
> + local isPropCmd=
> + [[ ${COMP_WORDS[1]} == @($propCmds) ]] && isPropCmd=1
> +
> + # provide allowed property names after property commands
> + if [[ $COMP_CWORD == 2 && $isPropCmd ]]
> + then
This assumes the property name is given immediately after the subcommand, but
options are often given here instead.
> + COMPREPLY=( $( compgen -W "$allProps" -- $cur ) )
> + return 0
> + fi
> +
> + # is it a revision property?
> + local isRevProp=
> + [[ $isPropCmd && ${COMP_WORDS[2]} == @(${revProps// /|}) ]] && \
> + isRevProp=1
> +
> + # possible completion values for properties
> + local values="\' --file"
> + if [[ $isPropCmd ]] ; then
> + case ${COMP_WORDS[2]} in
> + svn:keywords)
> + values="Id Rev URL Date Author \'"
If the idea of that single quote is to suggest to the user that they may wish
to type a shell quoting character to start a list of keywords, I don't think
that is the sort of thing completion should be doing.
> + ;;
> + svn:executable|svn:needs-lock)
> + values='1'
The canonical value for these is "*", not "1".
> + ;;
> + svn:eol-style)
> + values='native LF CR CRLF'
> + ;;
> + svn:mime-type)
> + values='text/plain text/html text/xml text/rtf text/ \
> + image/png image/gif image/jpeg image/tiff image/ \
> + audio/ video/ application/'
For the last three, Bash appends a space to the completion. Can we stop it
doing that? It must be possible, because it doesn't append a space after a
directory name.
For "text/" and "image/" it doesn't, because there are alternative completions.
So one crude way would be to make sure there is at least one alternative
completion in each category. "application/octet-stream" is an obvious one.
> + ;;
> + esac
> + fi
> +
> + # force --revprop option after revision properties
> + if [[ $COMP_CWORD == 3 && $isRevProp ]]
> + then
> + COMPREPLY=( $( compgen -W '--revprop' -- $cur ) )
> + return 0
> + fi
> +
> + # force --revision option after --revprop on revision properties
> + if [[ $COMP_CWORD == 4 && $isRevProp &&
> + ${COMP_WORDS[3]} = '--revprop' ]]
> + then
> + COMPREPLY=( $( compgen -W '--revision' -- $cur ) )
> + return 0
> + fi
> +
> + # handle pset values for revprops
> + if [[ $isRevProp && $COMP_CWORD == 6 &&
> + ${COMP_WORDS[1]} == @($psCmds) &&
> + ${COMP_WORDS[3]} = '--revprop' &&
> + ${COMP_WORDS[4]} = '--revision' ]]
It looks like this makes far too many assumptions about the order in which the
user specifies the options and parameters. It really needs to be more flexible
if it is to be useful to people other than yourself.
> + then
> + COMPREPLY=( $( compgen -W "$values" -- $cur ) )
> + return 0
> + fi
> +
> + # handle pset values for non rev props
> + if [[ $COMP_CWORD == 3 && ! $isRevProp &&
> + ${COMP_WORDS[1]} == @($psCmds) ]]
> + then
> + COMPREPLY=( $( compgen -W "$values" -- $cur ) )
> + return 0
> + fi
> +
> + # if not typing an option,
> + # or if the previous option required a parameter,
> + # then fallback on ordinary filename expansion
> helpCmds='help|--help|h|\?'
> if [[ ${COMP_WORDS[1]} != @($helpCmds) ]] && \
> [[ "$cur" != -* ]] || \
> @@ -126,22 +235,25 @@
> cmdOpts="$mOpts $rOpts $qOpts --force --editor-cmd $pOpts"
> ;;
> propdel|pdel|pd)
> - cmdOpts="$qOpts -R --recursive $rOpts --revprop $pOpts"
> + cmdOpts="$qOpts -R --recursive $rOpts $pOpts"
> + [[ $isRevProp ]] && cmdOpts="$cmdOpts --revprop"
This means "--revprop" is not suggested until AFTER the user has typed the
property name. I usually put it BEFORE the property name. Is there any harm
in it being allowed before the user types a property name?
I think what you intended is that if the user types the name of a non-revision
property, then you don't want "--revprop" to be suggested. Perhaps you could
rewrite this bit of code to do exactly that.
> ;;
> propedit|pedit|pe)
> - cmdOpts="$rOpts --revprop --encoding --editor-cmd $pOpts \
> - --force"
> + cmdOpts="$rOpts --encoding --editor-cmd $pOpts --force"
> + [[ $isRevProp ]] && cmdOpts="$cmdOpts --revprop"
> ;;
> propget|pget|pg)
> - cmdOpts="-R --recursive $rOpts --revprop --strict $pOpts"
> + cmdOpts="-R --recursive $rOpts --strict $pOpts"
> + [[ $isRevProp ]] && cmdOpts="$cmdOpts --revprop"
> ;;
> proplist|plist|pl)
> cmdOpts="-v --verbose -R --recursive $rOpts --revprop $qOpts \
> $pOpts"
> ;;
> propset|pset|ps)
> - cmdOpts="-F --file $qOpts --targets -R --recursive --revprop \
> + cmdOpts="-F --file $qOpts --targets -R --recursive \
> --encoding $pOpts $rOpts --force"
> + [[ $isRevProp ]] && cmdOpts="$cmdOpts --revprop"
> ;;
> resolved)
> cmdOpts="--targets -R --recursive $qOpts"
> @@ -168,11 +280,16 @@
> esac
>
> cmdOpts="$cmdOpts --help -h --config-dir"
> + local prev=''
>
> # take out options already given
> - for (( i=2; i<=$COMP_CWORD-1; ++i )) ; do
> - opt=${COMP_WORDS[$i]}
> + for opt in $COMP_WORDS
> + do
> + # skip if the previous option required a parameter
> + [[ $prev == @($optsParam) ]] && { prev=$opt ; continue ; }
This sets "prev" to the value of the parameter, which could be anything,
including something that looks like a Subversion option but isn't. For
example, the parameter to "-x" will typically look like a Subversion option.
So set "prev" to the empty string instead.
> + prev=$opt
>
> + # remove leading dashes and arguments
> case $opt in
> --*) optBase=${opt/=*/} ;;
> -*) optBase=${opt:0:2} ;;
> @@ -207,11 +324,6 @@
> cmdOpts=${cmdOpts/ -F / }
> ;;
> esac
> -
> - # skip next option if this one requires a parameter
> - if [[ $opt == @($optsParam) ]] ; then
> - ((++i))
> - fi
> done
>
> COMPREPLY=( $( compgen -W "$cmdOpts" -- $cur ) )
So, in conclusion, I like the idea but it needs a bit more work.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 8 17:28:58 2005