Fabien COELHO wrote:
>
> please find attached yet another patch to improve the bash completion.
> It addresses most comments done on the 3 previous submissions. I'm
> pretty happy with the result. I think it is a significant improvement
> over the current version, and it looks good enough for me.
There is a lot of good stuff in here and it is time I committed it, but I wish
you could stop adding new features to it at the same time as fixing bugs. Each
new feature brings more bugs.
~/src/subversion> svn pset svn:log --revprop --revision 10 --password _
<TAB>
svn completion: give your secret string for the connection
_
So we've got a new trick: displaying prompt messages. But after displaying the
message it should re-display the command so that you can continue editing it,
and here it doesn't, it just displays a space character on the next line. You
_can_ continue editing the command, but can't see what you're doing except for
simple additional typing at the end. (I'm using "GNU bash, version
3.00.16(1)-release" in Konsole.)
I find that message unnecessary and ugly, because it doesn't seem to help the
user in any significant way. It only helps in a general way which would also
apply to many other commands and options that take arguments: "svn co <TAB>" ->
"type a URL or an option here". The completion would really be better without
these messages.
By the way, I think there is a way to make Bash not do the default filename
completion when you return an empty list: remove "-o default" from the
"complete" command. If you do that, then you will need to return an array
generated by something like "compgen -o default" in the cases where a filename
is allowed. I think that is probably too complex to do in this patch. Let's
commit this patch and then you can consider doing that in another patch if you
are interested.
One other thing. I don't like this script using environment variables named
"SVN_*" because those will be confused with and may conflict with variables
that SVN uses, such as SVN_EDITOR. (I know the current set of variables
doesn't conflict, but both the ones used by this script and the ones used by
Subversion may change in the future.) I suggest changing the prefix to
"SVN_BASH_". You don't need to submit another version just to do that; I can
do it when I commit it if you agree that would be OK.
So, if you will just remove those messages (for password, username and too many
arguments) and replace them with something simple that beeps or does nothing or
just allows the default filename completion (I know you didn't like that but at
least it's simple and expected and usually beeps anyway), and fix the bugs
below, I'll commit this good work.
- Julian
> + # maximum number of additionnal arguments expected in various forms
> + case $cmd in
> + merge)
> + nExpectArgs=3
> + ;;
> + diff|di)
> + [[ ! $hasRevisionOpt ]] && nExpectArgs=2
That's wrong for "diff": any number of arguments is OK in a plain command like:
svn diff utils.c utils.h main.c
> + ;;
> + copy|cp|move|mv|rename|ren|export|import)
> + nExpectArgs=2
> + ;;
> + switch|sw)
> + [[ ! $hasRelocate ]] && nExpectArgs=2
That's right in intention (switch without --relocate should have a maximum of
two arguments) but it doesn't work: it also restricts switch WITH --relocate:
svn switch --relocate file:///tmp/repos file:///tmp/repos2 a_
<TAB>
svn completion: nothing more is expected!
_
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Nov 19 14:22:06 2005