-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 18/12/2012 18:53, Junio C Hamano ha scritto:
> [jch: cc'ed git-completion experts to review implementation details]
>
> Manlio Perillo <[email protected]> writes:
>
>> The git-completion.bash script did not implemented full, git aware,
>> support for completion, for git commands that operate on files within
>> the current working directory or the index.
>>
>> For these commands, only long options completion was available.
>
> I find the "long options completion" is a misleading phrase. It
> sounds as if you changed the current completion that does not
> complete "git commit -<TAB>" but does "git commit --<TAB>" to
> complete the short options (e.g. "git commit -c"), but I do not
> think that is the topic of this patch.
>
It does not sound misleading to me.
I'm saying that the git-completion.bash script only implemented
completion for long options, but not for file names in the current
working directory.
Do you think I should rewrite the subject and the log message introduction?
As an example, something like this in the subject:
git-completion.bash: improve some git commands completion
and in the message:
The git-completion.bash script did not implemented full, git aware,
support for completion, for git commands that operate on files within
the current working directory or the index.
As an example:
...
I'm still not fully satisfied with it, however.
It still requires reading the full message to understand the changes
implemented.
>> As an example:
>>
>> git add <TAB>
>>
>> will suggest all files in the current working directory, including
>> ignored files and files that have not been modified.
>>
>> Full support for completion is now implemented, for git commands where
>
> s/Full.*implemented/Support more comprehensive completion/; or
> something, talking in the imperative mood (i.e. as if you are giving
> the order to the codebase to do something).
>
Ok.
>> the non-option arguments always refer to paths within the current
>> working directory or the index, as the follow:
>>
>> * the path completion for the "git mv", "git rm" and "git ls-tree"
>> commands will suggest all cached files.
>
> I thought you dropped "git mv" in this round.
>
Well, no.
But the current implementation should not cause problems.
Also note that I added support for ls-files, too.
There are some XXX marks in the code, but I think that the changes
always improve the old behaviour.
> [...]
>> For all affected commands, completion will always stop at directory
>> boundary. Only standard ignored files are excluded, using the
>> --exclude-standard option of the ls-files command.
>
> I read "always stop at directory boundary" to mean that
>
> git cmd t<TAB>
>
> will give us "t/ tag.c" (assuming there is a new or modified file in
> t/ and tag.c is the only modified file at the root level that begins
> with "t") and then
>
> git cmd t/<TAB>
>
> will likewise show the files and top-level subdirectories within t/
> directory. That would be great.
>
Yes, this is how it works, bugs excluded (I'm not a bash/perl expert).
> [...]
>> +# Perl filter used to process path list returned by ls-files and
>> +# diff-index --name-only commands, in order to list file names
>> +# relative to a specified directory, and append a slash to directory
>> +# names.
>> +# The script expects the prefix path in the "pfx" environ variable.
>> +# The output must be processed with the uniq filter, to remove
>> +# duplicate directories.
>> +# XXX remove duplicates in the Perl script ?
>
> Surely, that will remove one fork/exec with pipeline. I am not sure
> what the performance implication of using Perl here, but because we
> do not have to stick to POSIX shell in this file, the completion
> experts would be able to help rewriting this logic as a pure bash
> script.
>
Ok. I'll wait for a review from git-completion experts.
Note that the performance is the reason why I suggested, in a previous
email, that git should have some more options to format data in custom ways.
As an example, there is no way to tell ls-files to not recurse
directories, and there is no way to also get the file type.
A --no-recurse option, and a change in the code to make, as an example
git ls-files --stage --modified
to honor the --modified option, will probably make it possible to use a
simple sed filter (there is still the problem that, unlike ls-tree,
ls-files shows the complete file path).
> [...]
>> +__git_files ()
>> +{
>> + local dir="$(__gitdir)" flags="-${1}"
>> +
>> + if [ -d "$dir" ]; then
>> + git --git-dir="$dir" ls-files --exclude-standard ${flags}
>> ${pfx} \
>> + | pfx=$2 perl -ne "${__git_index_file_list_filter}" \
>> + | uniq
>
> This is purely a style thing (note that style suggestions are not
> optional), but
>
> the data source command |
> a filter command |
> another filter command
>
> is easier to read and can be spelled without the backslash. The
> same comment applies to git-commit-files as well.
>
I agree.
But I was copying the style currently used in the script
(see the __git_complete_revlist_file function).
Note that I plan to do a small code refactorization, since I need the
ls-tree support code from __git_complete_revlist_file function for a
future change. I can fix these style issues in that patch.
I plan to improve completion support for checkout and reset commands,
too (currently only the commit/tree-ish argument is autocompleted, but
not the path).
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDQvDIACgkQscQJ24LbaUTT9wCgh6jtbhdQ7GNIkqCq34QjXdIs
w9QAnjz2VjPFm4n2ICkcWWEsqWDWM+Hm
=8/Ad
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
