Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit
On Tue, Jan 16, 2018 at 11:36 AM, Nguyễn Thái Ngọc Duy wrote: > I noticed --recurse-submodules was missing from git-grep complete > list. Then I found a couple more should be on the list as well and > many more in future may face the same faith. Perhaps this helps remedy > this situation? > > This lets us extract certain information from git commands and feed it > directly to git-completion.bash. Now long options by default will > be complete-able (which also means it's the reviewer's and coder's > responsibility to add "no complete" flag appropriately) but I think > the number of new dangerous options will be much fewer than > completeable ones. > > This is not really a new idea. Python has argcomplete that does more > or less the same thing. This has come up before for Git as well, see: https://public-inbox.org/git/1334140165-24958-1-git-send-email-bebar...@gmail.com/T/#u I see that your approach solves one of the shortcomings of those older patches, namely it makes possible to omit dangerous options from completion. Great. I also see that you want to invoke git in a subshell every time the user attempts to complete an --option. Not so great, at least for Windows users. That older thread contains a few ideas about how to do it only once by lazy-initializing a variable for each command to hold its options. > This is just a proof of concept. More commands should be converted of > course if it's a good thing to do. > > Nguyễn Thái Ngọc Duy (2): > parse-options: support --git-completion-helper > git-completion: use --git-completion-helper > > builtin/grep.c | 13 +++- > contrib/completion/git-completion.bash | 16 +-- > parse-options.c| 37 > ++ > parse-options.h| 14 - > 4 files changed, 55 insertions(+), 25 deletions(-) > > -- > 2.15.1.600.g899a5f85c6 >
Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit
On Wed, Jan 17, 2018 at 7:51 AM, SZEDER Gábor wrote: > On Tue, Jan 16, 2018 at 11:36 AM, Nguyễn Thái Ngọc Duy > wrote: >> I noticed --recurse-submodules was missing from git-grep complete >> list. Then I found a couple more should be on the list as well and >> many more in future may face the same faith. Perhaps this helps remedy >> this situation? >> >> This lets us extract certain information from git commands and feed it >> directly to git-completion.bash. Now long options by default will >> be complete-able (which also means it's the reviewer's and coder's >> responsibility to add "no complete" flag appropriately) but I think >> the number of new dangerous options will be much fewer than >> completeable ones. >> >> This is not really a new idea. Python has argcomplete that does more >> or less the same thing. > > This has come up before for Git as well, see: > > > https://public-inbox.org/git/1334140165-24958-1-git-send-email-bebar...@gmail.com/T/#u Thanks! I did search the archive but couldn't find this one. > > I see that your approach solves one of the shortcomings of those older > patches, namely it makes possible to omit dangerous options from > completion. Great. > > I also see that you want to invoke git in a subshell every time the user > attempts to complete an --option. Not so great, at least for Windows > users. That older thread contains a few ideas about how to do it only > once by lazy-initializing a variable for each command to hold its > options. Noted. I see you also pointed out the problem with running commands like "git-status" without a repository. I'll try to address this too if possible (I'm thinking about making struct options[] available outside cmd_*(); then we could handle it more like "git --version" which always works) -- Duy
Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit
Actually I forgot another option. What if we automate updating the
script at "compile" time instead of calling git at run time? E.g. with
something like below, a contributor could just run
make update-completion
then add git-completion.bash changes to the same patch that introduces
new options. If they forget, Junio could always run this near -rc0.
I know this output is a bit ugly. I probably could try to make the
update work with wrapped __gitcomp lines to be friendlier to human
eyes.
-- 8< --
diff --git a/Makefile b/Makefile
index 1a9b23b679..05eb7c8742 100644
--- a/Makefile
+++ b/Makefile
@@ -2834,3 +2834,5 @@ cover_db: coverage-report
cover_db_html: cover_db
cover -report html -outputdir cover_db_html cover_db
+update-completion:
+ contrib/completion/update.sh contrib/completion/git-completion.bash
./git
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 3683c772c5..e8c224f486 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1585,21 +1585,7 @@ _git_grep ()
case "$cur" in
--*)
- __gitcomp "
- --cached
- --text --ignore-case --word-regexp --invert-match
- --full-name --line-number
- --extended-regexp --basic-regexp --fixed-strings
- --perl-regexp
- --threads
- --files-with-matches --name-only
- --files-without-match
- --max-depth
- --count
- --and --or --not --all-match
- --break --heading --show-function --function-context
- --untracked --no-index
- "
+ __gitcomp_auto grep "--cached --text --ignore-case
--word-regexp --invert-match --full-name --line-number --extended-regexp
--basic-regexp --fixed-strings --perl-regexp --threads --files-with-matches
--name-only --files-without-match --max-depth --count --and --or --not
--all-match --break --heading --show-function --function-context --untracked
--no-index"
return
;;
esac
diff --git a/contrib/completion/update.sh b/contrib/completion/update.sh
new file mode 100755
index 00..99c4841152
--- /dev/null
+++ b/contrib/completion/update.sh
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+file="$1"
+git="$2"
+
+grep __gitcomp_auto "$file" | while read a cmd b; do
+sed -i "s/\\(.*$a $cmd \).*/\\1$("$git" $cmd --git-completion-helper)/"
"$file"
+done
-- 8< --
On Wed, Jan 17, 2018 at 04:16:22PM +0700, Duy Nguyen wrote:
> On Wed, Jan 17, 2018 at 7:51 AM, SZEDER Gábor wrote:
> > On Tue, Jan 16, 2018 at 11:36 AM, Nguyễn Thái Ngọc Duy
> > wrote:
> >> I noticed --recurse-submodules was missing from git-grep complete
> >> list. Then I found a couple more should be on the list as well and
> >> many more in future may face the same faith. Perhaps this helps remedy
> >> this situation?
> >>
> >> This lets us extract certain information from git commands and feed it
> >> directly to git-completion.bash. Now long options by default will
> >> be complete-able (which also means it's the reviewer's and coder's
> >> responsibility to add "no complete" flag appropriately) but I think
> >> the number of new dangerous options will be much fewer than
> >> completeable ones.
> >>
> >> This is not really a new idea. Python has argcomplete that does more
> >> or less the same thing.
> >
> > This has come up before for Git as well, see:
> >
> >
> > https://public-inbox.org/git/1334140165-24958-1-git-send-email-bebar...@gmail.com/T/#u
>
> Thanks! I did search the archive but couldn't find this one.
>
> >
> > I see that your approach solves one of the shortcomings of those older
> > patches, namely it makes possible to omit dangerous options from
> > completion. Great.
> >
> > I also see that you want to invoke git in a subshell every time the user
> > attempts to complete an --option. Not so great, at least for Windows
> > users. That older thread contains a few ideas about how to do it only
> > once by lazy-initializing a variable for each command to hold its
> > options.
>
> Noted.
>
> I see you also pointed out the problem with running commands like
> "git-status" without a repository. I'll try to address this too if
> possible (I'm thinking about making struct options[] available outside
> cmd_*(); then we could handle it more like "git --version" which
> always works)
> --
> Duy
Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit
On Wed, Jan 17, 2018 at 10:34 AM, Duy Nguyen wrote: > Actually I forgot another option. What if we automate updating the > script at "compile" time instead of calling git at run time? E.g. with > something like below, a contributor could just run > > make update-completion > > then add git-completion.bash changes to the same patch that introduces > new options. If they forget They inevitably will :) If contributors have to remember something anyway, then they might as well remember to update the completion script in the first place. Another alternative would be to extend t9902 with (preferably auto-generated) tests to compare the output of 'git $cmd --git-completion-helper' with 'run_completion "git $cmd --"'. Then contributors wouldn't have to remember anything, because everyone runs the full test suite every time anyway, right? However, that would result in some code churn initially, because I suspect the options are listed in different order in the command and in the completion script. All in all I don't think it would trump getting all --options straight from the commands themselves.
Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit
On Mon, Jan 22, 2018 at 07:03:24PM +0100, SZEDER Gábor wrote: > On Wed, Jan 17, 2018 at 10:34 AM, Duy Nguyen wrote: > > Actually I forgot another option. What if we automate updating the > > script at "compile" time instead of calling git at run time? E.g. with > > something like below, a contributor could just run > > > > make update-completion > > > > then add git-completion.bash changes to the same patch that introduces > > new options. If they forget > > They inevitably will :) OK. We go with the first option then (with caching to reduce overhead on Windows). I'm not going to bother you with actual code changes. The diff of completable options looks like below. It would be great if people could check if some options should NOT be completable for some reason. A couple points - Ignore options that are removed in the diff. They are removed for a reason which I will explain when real patches come. - There are regressions where --foo= becomes --foo, I hope this is ok for now. We can tweak this later. - In some commands you can complete both --foo and --foo= (now it's just one form, --foo). I would argue that it's pointless. It's no big deal to type '=' (or a space) after we have completed --foo. - I feel --force is not treated the same way everywhere. But I guess that's ok since "forcing" in some command context may be less severe than others. -- 8< -- diff --git a/git-add.txt b/git-add.txt index 1c249a3..2693cc1 100644 --- a/git-add.txt +++ b/git-add.txt @@ -1,10 +1,15 @@ +--all --chmod= --dry-run --edit --force --ignore-errors +--ignore-missing +--ignore-removal --intent-to-add --interactive --patch --refresh +--renormalize --update +--verbose diff --git a/git-am.txt b/git-am.txt index b0082bf..552dc96 100644 --- a/git-am.txt +++ b/git-am.txt @@ -1,12 +1,24 @@ --3way --committer-date-is-author-date +--directory +--exclude +--gpg-sign --ignore-date --ignore-space-change --ignore-whitespace +--include --interactive --keep +--keep-cr +--keep-non-patch +--message-id +--no-keep-cr --no-utf8 +--patch-format +--quiet +--reject +--resolvemsg= --scissors --signoff --utf8 ---whitespace= +--whitespace diff --git a/git-apply.txt b/git-apply.txt index 6bf4c2f..71d53d2 100644 --- a/git-apply.txt +++ b/git-apply.txt @@ -1,13 +1,17 @@ +--3way +--allow-overlap --apply +--build-fake-ancestor= --cached --check ---directory= ---exclude= +--directory +--exclude --ignore-space-change --ignore-whitespace --inaccurate-eof +--include --index ---index-info +--no-add --no-add --numstat --recount @@ -17,4 +21,4 @@ --summary --unidiff-zero --verbose ---whitespace= +--whitespace diff --git a/git-branch.txt b/git-branch.txt index 69594e3..9d308aa 100644 --- a/git-branch.txt +++ b/git-branch.txt @@ -1,10 +1,14 @@ ---abbrev= +--abbrev +--all --color --column --contains --copy +--create-reflog --delete --edit-description +--format= +--ignore-case --list --merged --move @@ -15,9 +19,10 @@ --no-merged --no-track --points-at +--quiet --remotes --set-upstream-to= ---sort= +--sort --track --unset-upstream --verbose diff --git a/git-checkout.txt b/git-checkout.txt index 493a1fe..75f19d2 100644 --- a/git-checkout.txt +++ b/git-checkout.txt @@ -1,12 +1,14 @@ --conflict= --detach +--ignore-other-worktrees --ignore-skip-worktree-bits --merge --no-recurse-submodules --no-track ---orphan +--orphan= --ours --patch +--progress --quiet --recurse-submodules --theirs diff --git a/git-cherry-pick.txt b/git-cherry-pick.txt index 39ba895..f8cdbce 100644 --- a/git-cherry-pick.txt +++ b/git-cherry-pick.txt @@ -1,5 +1,14 @@ +--abort +--allow-empty +--allow-empty-message +--continue --edit +--ff +--gpg-sign +--keep-redundant-commits --mainline --no-commit +--quit --signoff --strategy= +--strategy-option diff --git a/git-clean.txt b/git-clean.txt index 40407f7..10c6155 100644 --- a/git-clean.txt +++ b/git-clean.txt @@ -1,2 +1,4 @@ --dry-run +--exclude +--interactive --quiet diff --git a/git-clone.txt b/git-clone.txt index f6e892b..1b6a4da 100644 --- a/git-clone.txt +++ b/git-clone.txt @@ -1,18 +1,29 @@ --bare ---branch ---depth +--branch= +--config +--depth= +--dissociate +--ipv4 +--ipv6 +--jobs= --local --mirror --no-checkout --no-hardlinks --no-single-branch --no-tags ---origin +--origin= +--progress --quiet --recurse-submodules --reference +--reference-if-able +--separate-git-dir= +--shallow-exclude +--shallow-since= --shallow-submodules --shared --single-branch --template= ---upload-pack +--upload-pack= +--verbose diff --git a/git-commit.txt b/git-commit.txt index 2f98a59..337a57e 100644 --- a/git-commit.txt +++ b/git-commit.txt @@ -1,20 +1,26 @@ --all ---allow-empty --amend --author= +--branch --cleanup= ---date +--date= --dry-run --edit --file= --fixup= +--gpg-sign --include --interactive ---message= +--long +--message --no-edit +--no-post-rewrite --no-verify +--no-verify +--null --only --patch +--porcelain --quiet --reedit-message= -
Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit
Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit
Sorry mistyped 'y', please ignore. On Thu, Apr 11, 2019 at 6:10 PM Duy Nguyen wrote: > > -- Duy
