[PATCH/RFC 0/2] Automate updating git-completion.bash a bit

[PATCH/RFC 0/2] Automate updating git-completion.bash a bit

[Nguyễn Thái Ngọc Duy]

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 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

[PATCH 1/2] parse-options: support --git-completion-helper

[Nguyễn Thái Ngọc Duy]

This option is designed to be used by git-completion.bash. For many
simple cases, what we do in there is usually

    __gitcomp "lots of completion options"

which has to be manually updated when a new user-visible option is
added. With support from parse-options, we can write

    __gitcomp "$(git command --git-completion-helper)"

and get that list directly from the parser for free. Dangerous/Unpopular
options could be hidden with the new "NO_GITCOMP" flag.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 parse-options.c | 37 +++++++++++++++++++++++++++++++++++++
 parse-options.h |  6 ++++--
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index fca7159646..6c542f44cf 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -425,6 +425,35 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	parse_options_check(options);
 }
 
+static int show_gitcomp(struct parse_opt_ctx_t *ctx, const struct option *opts)
+{
+	for (; opts->type != OPTION_END; opts++) {
+		const char *suffix;
+
+		if (!opts->long_name)
+			continue;
+		if (opts->flags & (PARSE_OPT_HIDDEN | PARSE_OPT_NO_GITCOMP))
+			continue;
+
+		switch (opts->type) {
+		case OPTION_GROUP:
+			continue;
+		case OPTION_STRING:
+		case OPTION_FILENAME:
+		case OPTION_INTEGER:
+		case OPTION_MAGNITUDE:
+			if (!(opts->flags & PARSE_OPT_OPTARG))
+				suffix = "=";
+			break;
+		default:
+			suffix = "";
+		}
+		printf(" --%s%s", opts->long_name, suffix);
+	}
+	fputc('\n', stdout);
+	exit(0);
+}
+
 static int usage_with_options_internal(struct parse_opt_ctx_t *,
 				       const char * const *,
 				       const struct option *, int, int);
@@ -434,6 +463,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		       const char * const usagestr[])
 {
 	int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+	int internal_gitcomp = !(ctx->flags & PARSE_OPT_NO_INTERNAL_GITCOMP);
 	int err = 0;
 
 	/* we must reset ->opt, unknown short option leave it dangling */
@@ -455,6 +485,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		if (internal_help && ctx->total == 1 && !strcmp(arg + 1, "h"))
 			goto show_usage;
 
+		/* lone --git-completion-helper is asked by git-completion.bash */
+		if (internal_gitcomp && ctx->total == 1 &&
+		    !strcmp(arg + 1, "-git-completion-helper"))
+			goto show_git_comp;
+
 		if (arg[1] != '-') {
 			ctx->opt = arg + 1;
 			switch (parse_short_opt(ctx, options)) {
@@ -521,6 +556,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 	err = 1;
  show_usage:
 	return usage_with_options_internal(ctx, usagestr, options, 0, err);
+ show_git_comp:
+	return show_gitcomp(ctx, options);
 }
 
 int parse_options_end(struct parse_opt_ctx_t *ctx)
diff --git a/parse-options.h b/parse-options.h
index af711227ae..8a3389b05b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -27,7 +27,8 @@ enum parse_opt_flags {
 	PARSE_OPT_STOP_AT_NON_OPTION = 2,
 	PARSE_OPT_KEEP_ARGV0 = 4,
 	PARSE_OPT_KEEP_UNKNOWN = 8,
-	PARSE_OPT_NO_INTERNAL_HELP = 16
+	PARSE_OPT_NO_INTERNAL_HELP = 16,
+	PARSE_OPT_NO_INTERNAL_GITCOMP = 32
 };
 
 enum parse_opt_option_flags {
@@ -38,7 +39,8 @@ enum parse_opt_option_flags {
 	PARSE_OPT_LASTARG_DEFAULT = 16,
 	PARSE_OPT_NODASH = 32,
 	PARSE_OPT_LITERAL_ARGHELP = 64,
-	PARSE_OPT_SHELL_EVAL = 256
+	PARSE_OPT_SHELL_EVAL = 256,
+	PARSE_OPT_NO_GITCOMP = 512
 };
 
 struct option;
-- 
2.15.1.600.g899a5f85c6

Re: [PATCH 1/2] parse-options: support --git-completion-helper

[Jacob Keller]

On Tue, Jan 16, 2018 at 2:36 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> This option is designed to be used by git-completion.bash. For many
> simple cases, what we do in there is usually
>
>     __gitcomp "lots of completion options"
>
> which has to be manually updated when a new user-visible option is
> added. With support from parse-options, we can write
>
>     __gitcomp "$(git command --git-completion-helper)"
>
> and get that list directly from the parser for free. Dangerous/Unpopular
> options could be hidden with the new "NO_GITCOMP" flag.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Was this possibly avoided in the past due to being slower than simply
storing the list in the completion code itself?

I think this is a good idea myself.

Thanks,
Jake

> ---
>  parse-options.c | 37 +++++++++++++++++++++++++++++++++++++
>  parse-options.h |  6 ++++--
>  2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index fca7159646..6c542f44cf 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -425,6 +425,35 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
>         parse_options_check(options);
>  }
>
> +static int show_gitcomp(struct parse_opt_ctx_t *ctx, const struct option *opts)
> +{
> +       for (; opts->type != OPTION_END; opts++) {
> +               const char *suffix;
> +
> +               if (!opts->long_name)
> +                       continue;
> +               if (opts->flags & (PARSE_OPT_HIDDEN | PARSE_OPT_NO_GITCOMP))
> +                       continue;
> +
> +               switch (opts->type) {
> +               case OPTION_GROUP:
> +                       continue;
> +               case OPTION_STRING:
> +               case OPTION_FILENAME:
> +               case OPTION_INTEGER:
> +               case OPTION_MAGNITUDE:
> +                       if (!(opts->flags & PARSE_OPT_OPTARG))
> +                               suffix = "=";
> +                       break;
> +               default:
> +                       suffix = "";
> +               }
> +               printf(" --%s%s", opts->long_name, suffix);
> +       }
> +       fputc('\n', stdout);
> +       exit(0);
> +}
> +
>  static int usage_with_options_internal(struct parse_opt_ctx_t *,
>                                        const char * const *,
>                                        const struct option *, int, int);
> @@ -434,6 +463,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
>                        const char * const usagestr[])
>  {
>         int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
> +       int internal_gitcomp = !(ctx->flags & PARSE_OPT_NO_INTERNAL_GITCOMP);
>         int err = 0;
>
>         /* we must reset ->opt, unknown short option leave it dangling */
> @@ -455,6 +485,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
>                 if (internal_help && ctx->total == 1 && !strcmp(arg + 1, "h"))
>                         goto show_usage;
>
> +               /* lone --git-completion-helper is asked by git-completion.bash */
> +               if (internal_gitcomp && ctx->total == 1 &&
> +                   !strcmp(arg + 1, "-git-completion-helper"))
> +                       goto show_git_comp;
> +
>                 if (arg[1] != '-') {
>                         ctx->opt = arg + 1;
>                         switch (parse_short_opt(ctx, options)) {
> @@ -521,6 +556,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
>         err = 1;
>   show_usage:
>         return usage_with_options_internal(ctx, usagestr, options, 0, err);
> + show_git_comp:
> +       return show_gitcomp(ctx, options);
>  }
>
>  int parse_options_end(struct parse_opt_ctx_t *ctx)
> diff --git a/parse-options.h b/parse-options.h
> index af711227ae..8a3389b05b 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -27,7 +27,8 @@ enum parse_opt_flags {
>         PARSE_OPT_STOP_AT_NON_OPTION = 2,
>         PARSE_OPT_KEEP_ARGV0 = 4,
>         PARSE_OPT_KEEP_UNKNOWN = 8,
> -       PARSE_OPT_NO_INTERNAL_HELP = 16
> +       PARSE_OPT_NO_INTERNAL_HELP = 16,
> +       PARSE_OPT_NO_INTERNAL_GITCOMP = 32
>  };
>
>  enum parse_opt_option_flags {
> @@ -38,7 +39,8 @@ enum parse_opt_option_flags {
>         PARSE_OPT_LASTARG_DEFAULT = 16,
>         PARSE_OPT_NODASH = 32,
>         PARSE_OPT_LITERAL_ARGHELP = 64,
> -       PARSE_OPT_SHELL_EVAL = 256
> +       PARSE_OPT_SHELL_EVAL = 256,
> +       PARSE_OPT_NO_GITCOMP = 512
>  };
>
>  struct option;
> --
> 2.15.1.600.g899a5f85c6
>

Re: [PATCH 1/2] parse-options: support --git-completion-helper

[Duy Nguyen]

On Wed, Jan 17, 2018 at 1:25 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Tue, Jan 16, 2018 at 2:36 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> This option is designed to be used by git-completion.bash. For many
>> simple cases, what we do in there is usually
>>
>>     __gitcomp "lots of completion options"
>>
>> which has to be manually updated when a new user-visible option is
>> added. With support from parse-options, we can write
>>
>>     __gitcomp "$(git command --git-completion-helper)"
>>
>> and get that list directly from the parser for free. Dangerous/Unpopular
>> options could be hidden with the new "NO_GITCOMP" flag.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> Was this possibly avoided in the past due to being slower than simply
> storing the list in the completion code itself?

At least on linux I would not worry too much about performance (but
then I don't know what platforms this git-completion supports and
whether this may become real performance problems for them).
git-completion.bash already executes some heavy commands (getting refs
for completion for example), though this makes the script call git
much earlier.

There's one thing I will have to be careful about though, that to make
sure that --git-completion-helper runs even if the command itself is
not supposed to run, e.g. calling git-status without a worktree will
fail, but calling 'git status --git-completion-helper' should still
work in that condition.

-- 
Duy

Re: [PATCH 1/2] parse-options: support --git-completion-helper

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This option is designed to be used by git-completion.bash. For many
> simple cases, what we do in there is usually
>
>     __gitcomp "lots of completion options"
>
> which has to be manually updated when a new user-visible option is
> added. With support from parse-options, we can write
>
>     __gitcomp "$(git command --git-completion-helper)"
>
> and get that list directly from the parser for free. Dangerous/Unpopular
> options could be hidden with the new "NO_GITCOMP" flag.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  parse-options.c | 37 +++++++++++++++++++++++++++++++++++++
>  parse-options.h |  6 ++++--
>  2 files changed, 41 insertions(+), 2 deletions(-)

I do not know if you need PARSE_OPT_NO_INTERNAL_GITCOMP.  The only
way to trigger this is by passing a rather long option (intended to
be used only by scripts), so unlike PARSE_OPT_NO_INTERNAL_HELP which
was an escape hatch to free a short-and-sweet "-h" to be retargetted
by a selected few commands, there may not be need for it.

Some day when everybody uses parse-options, it would be trivially
correct to declare that this is the right approach ;-) I am not sure
how much of the current completion script's hardcoded option list
can be reduced with this approach with today's code, given that the
bigger ones (diff and log family) are not using parse-options API at
all, but for the ones that already do use parse-options, I think
this makes sense.

Thanks for a fun read ;-)

Re: [PATCH 1/2] parse-options: support --git-completion-helper

[Duy Nguyen]

On Wed, Jan 17, 2018 at 6:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> This option is designed to be used by git-completion.bash. For many
>> simple cases, what we do in there is usually
>>
>>     __gitcomp "lots of completion options"
>>
>> which has to be manually updated when a new user-visible option is
>> added. With support from parse-options, we can write
>>
>>     __gitcomp "$(git command --git-completion-helper)"
>>
>> and get that list directly from the parser for free. Dangerous/Unpopular
>> options could be hidden with the new "NO_GITCOMP" flag.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  parse-options.c | 37 +++++++++++++++++++++++++++++++++++++
>>  parse-options.h |  6 ++++--
>>  2 files changed, 41 insertions(+), 2 deletions(-)
>
> I do not know if you need PARSE_OPT_NO_INTERNAL_GITCOMP.  The only
> way to trigger this is by passing a rather long option (intended to
> be used only by scripts), so unlike PARSE_OPT_NO_INTERNAL_HELP which
> was an escape hatch to free a short-and-sweet "-h" to be retargetted
> by a selected few commands, there may not be need for it.

It's a copy/paste from internal_help. Because I was asking for
comments, not really aiming to submit these changes to git.git (yet),
I didn't bother to check further.

> Some day when everybody uses parse-options, it would be trivially
> correct to declare that this is the right approach ;-) I am not sure
> how much of the current completion script's hardcoded option list
> can be reduced with this approach with today's code, given that the
> bigger ones (diff and log family) are not using parse-options API at
> all, but for the ones that already do use parse-options, I think
> this makes sense.

Yeah yeah the horrible handle_revision_opt() and diff_opt_parse(). I
think I attempted to convert them two parse-options at some point, not
sure why I dropped it, maybe because target variables are in
dynamically allocated memory ('struct diff_options *' and 'struct
rev_info *') which does not fit well to how we usually initialize
'struct options[]'.
-- 
Duy

[PATCH 2/2] git-completion: use --git-completion-helper

[Nguyễn Thái Ngọc Duy]

The builtin command is updated to hide some options. The new options
that are complete-able are:

--after-context=
--all-match
--before-context=
--color
--context
--exclude-standard
--recurse-submodules
--textconv

--max-depth and --threads are already completable, but now it will
complete with the "=" suffix because these always take an argument.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/grep.c                         | 13 ++++++++-----
 contrib/completion/git-completion.bash | 16 +---------------
 parse-options.h                        |  8 +++++---
 3 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 3ca4ac80d8..8fe5560ee8 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -832,8 +832,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('L', "files-without-match",
 			&opt.unmatch_name_only,
 			N_("show only the names of files without match")),
-		OPT_BOOL('z', "null", &opt.null_following_name,
-			N_("print NUL after filenames")),
+		OPT_BOOL_F('z', "null", &opt.null_following_name,
+			   N_("print NUL after filenames"),
+			   PARSE_OPT_NO_GITCOMP),
 		OPT_BOOL('c', "count", &opt.count,
 			N_("show the number of matches instead of matching lines")),
 		OPT__COLOR(&opt.color, N_("highlight matches")),
@@ -884,9 +885,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		{ OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager,
 			N_("pager"), N_("show matching files in the pager"),
-			PARSE_OPT_OPTARG, NULL, (intptr_t)default_pager },
-		OPT_BOOL(0, "ext-grep", &external_grep_allowed__ignored,
-			 N_("allow calling of grep(1) (ignored by this build)")),
+			PARSE_OPT_OPTARG | PARSE_OPT_NO_GITCOMP,
+			NULL, (intptr_t)default_pager },
+		OPT_BOOL_F(0, "ext-grep", &external_grep_allowed__ignored,
+			   N_("allow calling of grep(1) (ignored by this build)"),
+			   PARSE_OPT_NO_GITCOMP),
 		OPT_END()
 	};
 
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3683c772c5..f0d9126fd6 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 "$(git grep --git-completion-helper)"
 		return
 		;;
 	esac
diff --git a/parse-options.h b/parse-options.h
index 8a3389b05b..2bd833a4c5 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -124,9 +124,11 @@ struct option {
 				      (h), PARSE_OPT_NOARG, NULL, (b) }
 #define OPT_COUNTUP(s, l, v, h)     { OPTION_COUNTUP, (s), (l), (v), NULL, \
 				      (h), PARSE_OPT_NOARG }
-#define OPT_SET_INT(s, l, v, h, i)  { OPTION_SET_INT, (s), (l), (v), NULL, \
-				      (h), PARSE_OPT_NOARG, NULL, (i) }
-#define OPT_BOOL(s, l, v, h)        OPT_SET_INT(s, l, v, h, 1)
+#define OPT_SET_INT_F(s, l, v, h, i, f) { OPTION_SET_INT, (s), (l), (v), NULL, \
+					  (h), PARSE_OPT_NOARG | (f), NULL, (i) }
+#define OPT_SET_INT(s, l, v, h, i)  OPT_SET_INT_F(s, l, v, h, i, 0)
+#define OPT_BOOL_F(s, l, v, h, f)   OPT_SET_INT_F(s, l, v, h, 1, f)
+#define OPT_BOOL(s, l, v, h)        OPT_BOOL_F(s, l, v, h, 0)
 #define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
 				      (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1}
 #define OPT_CMDMODE(s, l, v, h, i)  { OPTION_CMDMODE, (s), (l), (v), NULL, \
-- 
2.15.1.600.g899a5f85c6

Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit

[SZEDER Gábor]

On Tue, Jan 16, 2018 at 11:36 AM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> 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-bebarino@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

[Duy Nguyen]

On Wed, Jan 17, 2018 at 7:51 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Tue, Jan 16, 2018 at 11:36 AM, Nguyễn Thái Ngọc Duy
> <pclouds@gmail.com> 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-bebarino@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

[Duy Nguyen]

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 0000000000..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 <szeder.dev@gmail.com> wrote:
> > On Tue, Jan 16, 2018 at 11:36 AM, Nguyễn Thái Ngọc Duy
> > <pclouds@gmail.com> 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-bebarino@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

[SZEDER Gábor]

On Wed, Jan 17, 2018 at 10:34 AM, Duy Nguyen <pclouds@gmail.com> 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

[Duy Nguyen]

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 <pclouds@gmail.com> 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=
 --reset-author
@@ -22,8 +28,7 @@
 --short
 --signoff
 --squash=
+--status
 --template=
 --untracked-files
---untracked-files=
 --verbose
---verify
diff --git a/git-config.txt b/git-config.txt
index c4bcd1e..651fc27 100644
--- a/git-config.txt
+++ b/git-config.txt
@@ -1,15 +1,28 @@
 --add
+--blob=
+--bool
+--bool-or-int
+--edit
+--expiry-date
 --file=
 --get
 --get-all
+--get-color
+--get-colorbool
 --get-regexp
+--get-urlmatch
 --global
+--includes
+--int
 --list
 --local
 --name-only
+--null
+--path
 --remove-section
 --rename-section
 --replace-all
+--show-origin
 --system
 --unset
 --unset-all
diff --git a/git-describe.txt b/git-describe.txt
index 05eeb7a..275debb 100644
--- a/git-describe.txt
+++ b/git-describe.txt
@@ -1,4 +1,4 @@
---abbrev=
+--abbrev
 --all
 --always
 --broken
diff --git a/git-fsck.txt b/git-fsck.txt
index 9732ebf..3af9964 100644
--- a/git-fsck.txt
+++ b/git-fsck.txt
@@ -1,8 +1,12 @@
 --cache
+--connectivity-only
+--dangling
 --full
 --lost-found
 --name-objects
 --no-reflogs
+--progress
+--reflogs
 --root
 --strict
 --tags
diff --git a/git-gc.txt b/git-gc.txt
index 3b7ca6f..c08e60a 100644
--- a/git-gc.txt
+++ b/git-gc.txt
@@ -1,2 +1,3 @@
 --aggressive
 --prune
+--quiet
diff --git a/git-grep.txt b/git-grep.txt
index 6bec8f7..f7770be 100644
--- a/git-grep.txt
+++ b/git-grep.txt
@@ -1,9 +1,14 @@
+--after-context=
 --all-match
 --and
 --basic-regexp
+--before-context=
 --break
 --cached
+--color
+--context
 --count
+--exclude-standard
 --extended-regexp
 --files-with-matches
 --files-without-match
@@ -14,14 +19,17 @@
 --ignore-case
 --invert-match
 --line-number
---max-depth
+--max-depth=
 --name-only
 --no-index
 --not
 --or
 --perl-regexp
+--quiet
+--recurse-submodules
 --show-function
 --text
---threads
+--textconv
+--threads=
 --untracked
 --word-regexp
diff --git a/git-init.txt b/git-init.txt
index a7d3da6..aec6a35 100644
--- a/git-init.txt
+++ b/git-init.txt
@@ -1,5 +1,5 @@
 --bare
 --quiet
+--separate-git-dir=
 --shared
---shared=
 --template=
diff --git a/git-ls-files.txt b/git-ls-files.txt
index 04d6f08..6810eb0 100644
--- a/git-ls-files.txt
+++ b/git-ls-files.txt
@@ -1,20 +1,23 @@
 --abbrev
 --cached
+--debug
 --deleted
 --directory
+--empty-directory
+--eol
 --error-unmatch
---exclude=
---exclude-from=
---exclude-per-directory
+--exclude
+--exclude-from
 --exclude-per-directory=
 --exclude-standard
 --full-name
 --ignored
---ignored
 --killed
 --modified
 --no-empty-directory
 --others
+--recurse-submodules
+--resolve-undo
 --stage
 --unmerged
 --with-tree=
diff --git a/git-ls-remote.txt b/git-ls-remote.txt
index c58c80e..381384b 100644
--- a/git-ls-remote.txt
+++ b/git-ls-remote.txt
@@ -1,5 +1,7 @@
 --get-url
 --heads
+--quiet
 --refs
 --symref
 --tags
+--upload-pack=
diff --git a/git-mv.txt b/git-mv.txt
index c26c9e6..28ae674 100644
--- a/git-mv.txt
+++ b/git-mv.txt
@@ -1 +1,2 @@
 --dry-run
+--verbose
diff --git a/git-name-rev.txt b/git-name-rev.txt
index 7b1acde..1e5044a 100644
--- a/git-name-rev.txt
+++ b/git-name-rev.txt
@@ -1,3 +1,8 @@
 --all
+--always
+--exclude
+--name-only
+--refs
 --stdin
 --tags
+--undefined
diff --git a/git-notes_add.txt b/git-notes_add.txt
index 87cb06b..4c2dfc1 100644
--- a/git-notes_add.txt
+++ b/git-notes_add.txt
@@ -1,4 +1,5 @@
---file=
---message=
---reedit-message=
---reuse-message=
+--allow-empty
+--file
+--message
+--reedit-message
+--reuse-message
diff --git a/git-notes_append.txt b/git-notes_append.txt
index 87cb06b..4c2dfc1 100644
--- a/git-notes_append.txt
+++ b/git-notes_append.txt
@@ -1,4 +1,5 @@
---file=
---message=
---reedit-message=
---reuse-message=
+--allow-empty
+--file
+--message
+--reedit-message
+--reuse-message
diff --git a/git-notes_copy.txt b/git-notes_copy.txt
index 93aa025..d088b96 100644
--- a/git-notes_copy.txt
+++ b/git-notes_copy.txt
@@ -1 +1,2 @@
+--for-rewrite=
 --stdin
diff --git a/git-push.txt b/git-push.txt
index 1f4182f..5ea1e39 100644
--- a/git-push.txt
+++ b/git-push.txt
@@ -1,16 +1,24 @@
 --all
+--atomic
 --delete
 --dry-run
+--exec=
 --follow-tags
 --force
 --force-with-lease
---force-with-lease=
+--ipv4
+--ipv6
 --mirror
+--no-verify
+--porcelain
+--progress
 --prune
+--push-option
 --quiet
 --receive-pack=
---recurse-submodules=
+--recurse-submodules
 --repo=
 --set-upstream
+--signed
 --tags
 --verbose
diff --git a/git-remote_add.txt b/git-remote_add.txt
index aed6530..4889f31 100644
--- a/git-remote_add.txt
+++ b/git-remote_add.txt
@@ -1,6 +1,6 @@
 --fetch
---master
---mirror=
+--master=
+--mirror
 --no-tags
 --tags
 --track
diff --git a/git-replace.txt b/git-replace.txt
index 53cd1aa..d494653 100644
--- a/git-replace.txt
+++ b/git-replace.txt
@@ -3,3 +3,4 @@
 --format=
 --graft
 --list
+--raw
diff --git a/git-reset.txt b/git-reset.txt
index eafefbc..840cbae 100644
--- a/git-reset.txt
+++ b/git-reset.txt
@@ -1,6 +1,9 @@
 --hard
+--intent-to-add
 --keep
 --merge
 --mixed
 --patch
+--quiet
+--recurse-submodules
 --soft
diff --git a/git-revert.txt b/git-revert.txt
index abd5c1e..ffa0d80 100644
--- a/git-revert.txt
+++ b/git-revert.txt
@@ -1,7 +1,11 @@
+--abort
+--continue
 --edit
+--gpg-sign
 --mainline
 --no-commit
 --no-edit
+--quit
 --signoff
 --strategy=
---strategy-option=
+--strategy-option
diff --git a/git-show-branch.txt b/git-show-branch.txt
index e3c3096..37c9594 100644
--- a/git-show-branch.txt
+++ b/git-show-branch.txt
@@ -5,7 +5,7 @@
 --independent
 --list
 --merge-base
---more=
+--more
 --no-color
 --no-name
 --reflog
diff --git a/git-status.txt b/git-status.txt
index 2dce372..ed86ad5 100644
--- a/git-status.txt
+++ b/git-status.txt
@@ -1,10 +1,12 @@
 --branch
---column=
+--column
 --ignored
---ignore-submodules=
+--ignore-submodules
 --long
 --no-column
+--null
 --porcelain
 --short
---untracked-files=
+--show-stash
+--untracked-files
 --verbose
diff --git a/git-tag.txt b/git-tag.txt
index b983562..a5419db 100644
--- a/git-tag.txt
+++ b/git-tag.txt
@@ -1,18 +1,21 @@
 --annotate
---cleanup
+--cleanup=
+--color
 --column
 --contains
 --create-reflog
 --delete
---file
+--file=
 --force
+--format=
+--ignore-case
 --list
---local-user
+--local-user=
 --merged
 --message
 --no-contains
 --no-merged
 --points-at
 --sign
---sort=
+--sort
 --verify
diff --git a/git-worktree_add.txt b/git-worktree_add.txt
index 9dd7f92..3cb6a7e 100644
--- a/git-worktree_add.txt
+++ b/git-worktree_add.txt
@@ -1 +1,5 @@
+--checkout
 --detach
+--guess-remote
+--lock
+--track
diff --git a/git-worktree_lock.txt b/git-worktree_lock.txt
index a0bc06b..6ad59bb 100644
--- a/git-worktree_lock.txt
+++ b/git-worktree_lock.txt
@@ -1 +1 @@
---reason
+--reason=
-- 8< --
--
Duy

Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit

[Duy Nguyen]

Re: [PATCH/RFC 0/2] Automate updating git-completion.bash a bit

[Duy Nguyen]

Sorry mistyped 'y', please ignore.

On Thu, Apr 11, 2019 at 6:10 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
>


-- 
Duy