Re: [PATCH v4 03/13] commit-graph: create git-commit-graph builtin

[Derrick Stolee]

On 2/26/2018 11:25 AM, SZEDER Gábor wrote:

Teach git the 'commit-graph' builtin that will be used for writing and
reading packed graph files. The current implementation is mostly
empty, except for an '--object-dir' option.

Since 'git commit-graph' is a builtin command, it shouldn't show up in
completion when doing 'git co'.
Please squash in the patch below to make it so.

Furthermore, please have a look at
   
   https://public-inbox.org/git/20180202160132.31550-1-szeder@gmail.com/


for an other oneliner change.


diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 17929b0809..fafed13c06 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -841,6 +841,7 @@ __git_list_porcelain_commands ()
check-ref-format) : plumbing;;
checkout-index)   : plumbing;;
column)   : internal helper;;
+   commit-graph) : plumbing;;
commit-tree)  : plumbing;;
count-objects): infrequent;;
credential)   : credentials;;


Thanks for this, and the reminder. I made these changes locally, so they 
will be in v5.


-Stolee

Re: [PATCH v4 03/13] commit-graph: create git-commit-graph builtin

[SZEDER Gábor]

> Teach git the 'commit-graph' builtin that will be used for writing and
> reading packed graph files. The current implementation is mostly
> empty, except for an '--object-dir' option.

Since 'git commit-graph' is a builtin command, it shouldn't show up in
completion when doing 'git co'.
Please squash in the patch below to make it so.

Furthermore, please have a look at
  
  https://public-inbox.org/git/20180202160132.31550-1-szeder@gmail.com/

for an other oneliner change.


diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 17929b0809..fafed13c06 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -841,6 +841,7 @@ __git_list_porcelain_commands ()
check-ref-format) : plumbing;;
checkout-index)   : plumbing;;
column)   : internal helper;;
+   commit-graph) : plumbing;;
commit-tree)  : plumbing;;
count-objects): infrequent;;
credential)   : credentials;;

Re: [PATCH v4 03/13] commit-graph: create git-commit-graph builtin

[Derrick Stolee]

On 2/21/2018 1:58 PM, Junio C Hamano wrote:

Junio C Hamano  writes:


Derrick Stolee  writes:


+int cmd_commit_graph(int argc, const char **argv, const char *prefix)
+{
+   static struct option builtin_commit_graph_options[] = {
+   { OPTION_STRING, 'p', "object-dir", _dir,
+   N_("dir"),
+   N_("The object directory to store the graph") },

I have a suspicion that this was modeled after some other built-in
that has a similar issue (perhaps written long time ago), but isn't
OPT_STRING() sufficient to define this element these days?

Or am I missing something?


You are not. There are several places in this history of this patch 
where I was using old patterns because I was using old code as my model 
(places like 'index-pack').



Why squat on short-and-sweet "-p"?  For that matter, since this is
not expected to be end-user facing command anyway, I suspect that we
do not want to allocate a single letter option from day one, which
paints ourselves into a corner from where we cannot escape.


I'll drop all single-letter shortcuts.


I suspect that exactly the same comment applies to patches in this
series that add other subcommands (I just saw one in the patch for
adding 'write').



Thanks,
-Stolee

Re: [PATCH v4 03/13] commit-graph: create git-commit-graph builtin

[Junio C Hamano]

Junio C Hamano  writes:

> Derrick Stolee  writes:
>
>> +int cmd_commit_graph(int argc, const char **argv, const char *prefix)
>> +{
>> +static struct option builtin_commit_graph_options[] = {
>> +{ OPTION_STRING, 'p', "object-dir", _dir,
>> +N_("dir"),
>> +N_("The object directory to store the graph") },
>
> I have a suspicion that this was modeled after some other built-in
> that has a similar issue (perhaps written long time ago), but isn't
> OPT_STRING() sufficient to define this element these days?
>
> Or am I missing something?
>
> Why squat on short-and-sweet "-p"?  For that matter, since this is
> not expected to be end-user facing command anyway, I suspect that we
> do not want to allocate a single letter option from day one, which
> paints ourselves into a corner from where we cannot escape.

I suspect that exactly the same comment applies to patches in this
series that add other subcommands (I just saw one in the patch for
adding 'write').

Re: [PATCH v4 03/13] commit-graph: create git-commit-graph builtin

[Junio C Hamano]

Derrick Stolee  writes:

> +int cmd_commit_graph(int argc, const char **argv, const char *prefix)
> +{
> + static struct option builtin_commit_graph_options[] = {
> + { OPTION_STRING, 'p', "object-dir", _dir,
> + N_("dir"),
> + N_("The object directory to store the graph") },

I have a suspicion that this was modeled after some other built-in
that has a similar issue (perhaps written long time ago), but isn't
OPT_STRING() sufficient to define this element these days?

Or am I missing something?

Why squat on short-and-sweet "-p"?  For that matter, since this is
not expected to be end-user facing command anyway, I suspect that we
do not want to allocate a single letter option from day one, which
paints ourselves into a corner from where we cannot escape.