You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Fabien COELHO <fa...@coelho.net> on 2005/11/10 16:45:38 UTC

[PATCH] improved bash completion v2

Dear developers,

please find attached a patch to improve the bash completion. it addresses 
most comments done on the previous submission, at the price of a higher 
complexity. I'm pretty happy with the result. There may be bugs, but I 
think it is a significant improvement over the current version.


[[[
Improve bash auto-completion for svn command
  - there is a "real" parser to check for what is available
    and deduce what can be suggested further.
  - property names are completed.
  - --revprop and --revision options are suggested to revision properties
  - 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
  - argument values suggested to some other options

* tools/client-side/bash_completion
   add shell-intelligence to _svn completion function
]]]

-- 
Fabien

Re: [PATCH] improved bash completion v2

Posted by Fabien COELHO <fa...@coelho.net>.

> OK.  (For "pset" and "pedit", then, it shouldn't suggest "--revision" when 
> the property name is not a revprop, but I don't think that's very important.)

Indeed. Easy to fix.

>>>> +        COMPREPLY=( '' )
>> 
>> I do want to stop the file completion, because this case is triggered when 
>> no more arguments are expected (e.g. you're doing a cp and you already gave 
>> 2 arguments, whatever you're adding is garbage and should trigger an error 
>> of the completion, but not a standard file completion).
>
> That's fine if you can find a way to stop the completion, but what you're 
> doing doesn't stop it.  It stops _file name_ completion, but it doesn't stop 
> _completion_.  It just makes Bash think the word you have started is OK, and 
> considers it complete:

Ok,

I really want a beep, but compgen does not seem to provide a way to 
'really no choice'. Giving no choices is interpreted as 'try next completion'.

> Here's a kind-of-ugly idea that has the sort of effect you want:
>>   COMPREPLY=( 'bash_completion:' 'only two arguments wanted' )

Ugly but funny. I'll try a simpler:   echo -e "\nsome message\a" >&2
but it does not solve the inserted space.

-- 
Fabien.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] improved bash completion v2

Posted by Julian Foad <ju...@btopenworld.com>.
Fabien COELHO wrote:
> 
>> (4)
>>  $ svn pget prop file --revision 1 -
>>  <TAB>
>>  --config-dir       --non-interactive  --revision         -R
>>  --help             --password         --strict           -h
>>  --no-auth-cache    --recursive        --username         -r
[...]
>> and it doesn't list "--revprop" which is required when "--revision" is 
>> given.
> 
> Double no on this one :
> 
> --revprop is *not* required with --revision, you can ask for the value 
> of simple property on a file at a given revision in the past.

Oops - my mistake.  I wrote that with an example of "propset" (where 
"--revprop" is required iff --revision is given) and then "simplified" the example.

> They are both mandatory for revision properties... but "prop" is not.
> You can have that by adding it to the "SVN_REV_PROP" env variable.
> The default is to assume that it is a file property so only --revision
> is allowed.
> 
> Thus it is a feature, not a bug;-)

OK.  (For "pset" and "pedit", then, it shouldn't suggest "--revision" when the 
property name is not a revprop, but I don't think that's very important.)


>> (5)
>>  $ svn cp a b --_
>>  <TAB>
>>  $ svn cp a b -- _
>>
>> After both arguments have been given to a command like "cp", it no 
>> longer recognises options, and instead silently inserts a space.
> 
> Yes, idem this is a partially a *feature*, no options after the 
> arguments are supplied seems a sensible rule to me. It should have not 
> proposed '--'
> as a completion. I guess bash did it.

Well, it's inconsistent with how it behaves on other commands.  Only the 
two-argument commands suffer from this problem.  For other commands, your code 
offers the correct choice of options in this position.

   $ svn diff a b --_
   <TAB>
   --config-dir       --force            --no-auth-cache  [...]


>>> +        # some way to tell 'no completion'... is there a better one?
>>> +        COMPREPLY=( '' )
>>
>> This tells Bash that the only completion for this argument is the 
>> empty string.  I think it is more correct, and better in practice, to 
>> return an empty array; although Bash then uses filenames for 
>> completion, it is more likely to result in a beep (depending on what 
>> files are present) rather than a space.  (As an enhancement, maybe 
>> there is some way to tell Bash not to use file names for completion.)
> 
> I disagree.
> 
> I do want to stop the file completion, because this case is triggered 
> when no more arguments are expected (e.g. you're doing a cp and you 
> already gave 2 arguments, whatever you're adding is garbage and should 
> trigger an error of the completion, but not a standard file completion).

That's fine if you can find a way to stop the completion, but what you're doing 
doesn't stop it.  It stops _file name_ completion, but it doesn't stop 
_completion_.  It just makes Bash think the word you have started is OK, and 
considers it complete:

   $ svn cp a b c_
   <TAB>
   $ svn cp a b c _

Here's a kind-of-ugly idea that has the sort of effect you want:

>   # return some results that have no common prefix, so Bash won't
>   # use them, but when displayed (in alphabetical order) they form
>   # an error message
>   COMPREPLY=( 'bash_completion:' 'only two arguments wanted' )

   $ svn cp a b c_
   <TAB>
   <BEEP>
   <TAB>
   bash_completion:    only two arguments wanted
   $ svn cp a b c_

What happens is that when Bash gets a list of completions it _replaces_ the 
corrent word with the common prefix of all the completions, unless there is no 
common prefix in which case it complains and (on a second press of <TAB>) lists 
the completions.  This might help to explain the "svn:svn:log" problem.

This has the correct effect on the command line: it is unaltered and a beep is 
given.  You might not like the text that is printed.  It might be possible to 
make something almost invisible be printed, e.g. by returning two different 
invisible words: say a space and a tab.


- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] improved bash completion v2

Posted by Fabien COELHO <fa...@coelho.net>.

Dear Julian,


>>>  $ svn pe --revprop -r10 svn:l_
>>>  <TAB>
>>>  $ svn pe --revprop -r10 svn:svn:log
>
> I guess this is something to do with the way Bash uses the COMP_WORDBREAKS 
> variable.  Maybe it reads the variable as soon as the user requests 
> completion, not after the completion function returns.  Therefore, maybe, 
> when completing something that already begins with "svn:", instead of 
> returning "svn:..." with ":" removed from the word breaks list, we need to 
> put ":" back in the list (as usual) and return just the "..." part.

Maybe maybe maybe.

As I feel it is more a bash issue related to when COMP_WORDBREAKS is 
handled, and that I need to tweak it a little bit for prop names...
I'm not inclined to find a fix in the short term. I'm rather planing to 
caracterize the bug on a small example et send a bug report to bash 
people, and see what happens there. If it is proven to be a bash
feature rather than a bug, then I may look for a fix in the script.


> ...
> As I said, it's not just the first argument that has this quirk,
> it also happens after a subcommand.  You have fixed that first-argument 
> case, but not the general case:
>
>  $ svn log --quiet_
>  <TAB>
>  <BEEP>

Ok, I get it.

As the --quiet option is in the line, it is removed from the list of 
available options, so the completion does not work as it is not
suggested in the choices list, because it is already there;-)

I can fix this one.


> Here are some bugs that I didn't notice before:

Sigh;-)


> (4)
>  $ svn pget prop file --revision 1 -
>  <TAB>
>  --config-dir       --non-interactive  --revision         -R
>  --help             --password         --strict           -h
>  --no-auth-cache    --recursive        --username         -r
>
> It lists options including "--revision" and "-r" which should be excluded 
> because they have already been specified,

Humm.

I don't like having options after "free" arguments are supplied.
Otherwise, it leads to some ambiguities.

In your example, as a target file is given, it assumes that others are 
arguments, so '--revison' and '1' are file names. Ok, I admit that it 
is not what svn does. But I think that it is what it should do;-)


> and it doesn't list "--revprop" which is required when "--revision" is given.

Double no on this one :

--revprop is *not* required with --revision, you can ask for the value of 
simple property on a file at a given revision in the past.

They are both mandatory for revision properties... but "prop" is not.
You can have that by adding it to the "SVN_REV_PROP" env variable.
The default is to assume that it is a file property so only --revision
is allowed.

Thus it is a feature, not a bug;-)


> (5)
>  $ svn cp a b --_
>  <TAB>
>  $ svn cp a b -- _
>
> After both arguments have been given to a command like "cp", it no longer 
> recognises options, and instead silently inserts a space.

Yes, idem this is a partially a *feature*, no options after the arguments 
are supplied seems a sensible rule to me. It should have not proposed '--'
as a completion. I guess bash did it.


>> +	    # some way to tell 'no completion'... is there a better one?
>> +	    COMPREPLY=( '' )
>
> This tells Bash that the only completion for this argument is the empty 
> string.  I think it is more correct, and better in practice, to return an 
> empty array; although Bash then uses filenames for completion, it is more 
> likely to result in a beep (depending on what files are present) rather than 
> a space.  (As an enhancement, maybe there is some way to tell Bash not to use 
> file names for completion.)

I disagree.

I do want to stop the file completion, because this case is triggered when 
no more arguments are expected (e.g. you're doing a cp and you already 
gave 2 arguments, whatever you're adding is garbage and should trigger an 
error of the completion, but not a standard file completion).


> I hope you enjoy fixing bugs!  (I do.)

Moderatly, esp if I disagree that something is actually a bug
and I'm not getting paid for it;-)

I'll send a 4th version that should hopefully fix what I accept as bugs of
the script. For the features, they will stay there as far as my submission 
is concerned, but you'll be free to add your feature to it if you want;-)

-- 
Fabien.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] improved bash completion v2

Posted by Julian Foad <ju...@btopenworld.com>.
This is a review of v3, but based on the discussion about v2.

Fabien COELHO wrote:
> 
>> (1)
>>  $ svn pe --revprop -r10 svn:l_
>>  <TAB>
>>  $ svn pe --revprop -r10 svn:svn:log
>>
>> This doesn't happen every time, only when trying that completion for 
>> the first time after trying a different one.  Perhaps that indicates 
>> an uninitialised variable or similar.
> 
> Maybe. I'll try to spot the reason for it.

I guess this is something to do with the way Bash uses the COMP_WORDBREAKS 
variable.  Maybe it reads the variable as soon as the user requests completion, 
not after the completion function returns.  Therefore, maybe, when completing 
something that already begins with "svn:", instead of returning "svn:..." with 
":" removed from the word breaks list, we need to put ":" back in the list (as 
usual) and return just the "..." part.


>> (2)
>>  $ svn bl_ subversion/clients/cmdline/main.c
>>  <TAB>
>>  $ svn blank-log-msg.patch  subversion/clients/cmdline/main.c
>>
>> That's the name of a local file; it should complete the subcommand 
>> name "blame" instead.  The filename afterwards seems to confuse it.
> 
> Yep. I'm not sure the script is ok when completing inside a line rather
> than at the end. I have not tested that, so it must be wrong.

OK, you seem to have fixed completion within a line.


>> (3)
>>  $ svn --help_
>>  <TAB>
>>  <BEEP>
>>
>> Bash beeps, presumably because this script returns no results.  It 
>> should recognise that there is exactly one completion, and so Bash 
>> should quietly append a space.  The same occurs for any 
>> already-complete "-" or "--" option after a subcommand.
> 
> I can look into that. This case is a little particular because the first 
> argument is expected to be a subcommand, not an option.

As I said, it's not just the first argument that has this quirk, it also 
happens after a subcommand.  You have fixed that first-argument case, but not 
the general case:

   $ svn log --quiet_
   <TAB>
   <BEEP>


Here are some bugs that I didn't notice before:

(4)
   $ svn pget prop file --revision 1 -
   <TAB>
   --config-dir       --non-interactive  --revision         -R
   --help             --password         --strict           -h
   --no-auth-cache    --recursive        --username         -r

It lists options including "--revision" and "-r" which should be excluded 
because they have already been specified, and it doesn't list "--revprop" which 
is required when "--revision" is given.

(5)
   $ svn cp a b --_
   <TAB>
   $ svn cp a b -- _

After both arguments have been given to a command like "cp", it no longer 
recognises options, and instead silently inserts a space.

The space is due to this line:

 > +	    # some way to tell 'no completion'... is there a better one?
 > +	    COMPREPLY=( '' )

This tells Bash that the only completion for this argument is the empty string. 
  I think it is more correct, and better in practice, to return an empty array; 
although Bash then uses filenames for completion, it is more likely to result 
in a beep (depending on what files are present) rather than a space.  (As an 
enhancement, maybe there is some way to tell Bash not to use file names for 
completion.)


I hope you enjoy fixing bugs!  (I do.)

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] improved bash completion v2

Posted by Fabien COELHO <fa...@coelho.net>.
Dear Julian,

Thanks for the very active testing;-)

> This patch makes Bash suggest different completions in different 
> circumstances: in particular, if you request completion of an empty word and 
> there is some required part missing from the command, it only suggests the 
> missing thing.  I wasn't too sure about that idea to begin with, but it seems 
> to work rather well in practice.

Yes,

I think that directing the syntax to provide the mandatory parts helps.
The price is that options are actually parsed.


> I found some bugs:

Good;-)


> (1)
>  $ svn pe --revprop -r10 svn:l_
>  <TAB>
>  $ svn pe --revprop -r10 svn:svn:log
>
> This doesn't happen every time, only when trying that completion for the 
> first time after trying a different one.  Perhaps that indicates an 
> uninitialised variable or similar.

Maybe. I'll try to spot the reason for it.

> (2)
>  $ svn bl_ subversion/clients/cmdline/main.c
>  <TAB>
>  $ svn blank-log-msg.patch  subversion/clients/cmdline/main.c
>
> That's the name of a local file; it should complete the subcommand name 
> "blame" instead.  The filename afterwards seems to confuse it.

Yep. I'm not sure the script is ok when completing inside a line rather
than at the end. I have not tested that, so it must be wrong.

>  $ svn pset svn:mime-type ima_ www/images/jolt-2005.jpg
>  <TAB>
>  <BEEP>
>
> Similarly, here I expect the completion "image/", but it doesn't recognise 
> that I'm entering a MIME type.

Idem, it is within a line, not at the end of it.

> (3)
>  $ svn --help_
>  <TAB>
>  <BEEP>
>
> Bash beeps, presumably because this script returns no results.  It should 
> recognise that there is exactly one completion, and so Bash should quietly 
> append a space.  The same occurs for any already-complete "-" or "--" option 
> after a subcommand.

I can look into that. This case is a little particular because the first 
argument is expected to be a subcommand, not an option.


>> +	# commands which only expect two arguments
>> +	# diff has some variant with two arguments, but not all of them.
>> +	# merge/switch have two or three args variants.
>> +	twoArgsCmds='copy|cp|move|mv|export|import'
>
> "move" also has aliases "ren" and "rename".

Ok.

-- 
Fabien.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] improved bash completion v2

Posted by Julian Foad <ju...@btopenworld.com>.
First, a note for other readers:

This patch makes Bash suggest different completions in different circumstances: 
in particular, if you request completion of an empty word and there is some 
required part missing from the command, it only suggests the missing thing.  I 
wasn't too sure about that idea to begin with, but it seems to work rather well 
in practice.  For example, it knows that "svn:log" is a revision property:

(An underscore indicates the cursor.)

   $ svn ps svn:log _
   <TAB>
   $ svn ps svn:log --revprop _
   <TAB>
   $ svn ps svn:log --revprop --revision _
   10
   $ svn ps svn:log --revprop --revision 10 _
   <TAB>
   <BEEP>
   <TAB>
   '	--file

It is now suggesting that either a literal value (perhaps starting with a 
quote) or the "--file" option is needed.  However, if you want to put something 
else here you can:

   $ svn ps svn:log --revprop --revision 10 --p_
   <TAB>
   $ svn ps svn:log --revprop --revision 10 --password _
   <TAB>
   <BEEP>
   <TAB>
   '	--file

-------------------------------

I found some bugs:

(1)
   $ svn pe --revprop -r10 svn:l_
   <TAB>
   $ svn pe --revprop -r10 svn:svn:log

This doesn't happen every time, only when trying that completion for the first 
time after trying a different one.  Perhaps that indicates an uninitialised 
variable or similar.


(2)
   $ svn bl_ subversion/clients/cmdline/main.c
   <TAB>
   $ svn blank-log-msg.patch  subversion/clients/cmdline/main.c

That's the name of a local file; it should complete the subcommand name "blame" 
instead.  The filename afterwards seems to confuse it.

   $ svn pset svn:mime-type ima_ www/images/jolt-2005.jpg
   <TAB>
   <BEEP>

Similarly, here I expect the completion "image/", but it doesn't recognise that 
I'm entering a MIME type.


(3)
   $ svn --help_
   <TAB>
   <BEEP>

Bash beeps, presumably because this script returns no results.  It should 
recognise that there is exactly one completion, and so Bash should quietly 
append a space.  The same occurs for any already-complete "-" or "--" option 
after a subcommand.

-------------------------------

I haven't looked much at the code yet, but I saw one little thing:

> +	# commands which only expect two arguments
> +	# diff has some variant with two arguments, but not all of them.
> +	# merge/switch have two or three args variants.
> +	twoArgsCmds='copy|cp|move|mv|export|import'

"move" also has aliases "ren" and "rename".


- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org