Teach git annotate to use parse option instead of its own argv
cover Teach git annotate to use parse option instead of its own argv
The file builtIn/annotate.c was removed, letting git annotate use the cmd_blame directly.
===================
The following files were edited
===============================
--- Makefile --- git.c --- builtIn/annotate.c was removed
Abimbola (5):
remove-annotate: change cmd_annotate to cmd_blame edit the makefile
remove-annotate: comment out annotate BUILDIN_OBJS
remove-annotate: delete builtin annotate file fix unproper comment in Makefile
Makefile | 1 - annotate.txt | Bin 0 -> 163170 bytes blame.txt | Bin 0 -> 177882 bytes builtin/annotate.c | 22 ------- git.c | 143 ++++++++++++++++++++++++++------------------- 5 files changed, 82 insertions(+), 84 deletions(-) create mode 100644 annotate.txt create mode 100644 blame.txt delete mode 100644 builtin/annotate.c
base-commit: efd54442381a2792186abc994060b8f7dd8b834b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-442%2Fcraftword%2Fremove-annotate-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-442/craftword/remove-annotate-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/442
Signature · 1 line
gitgitgadget
PATCH 1/5 remove-annotate: change cmd_annotate to cmd_blame
From: Abimbola <[email protected]>
Changing this command is to remove the annotate.c file which does almost the same thing as blame.c. git annotate will invoc blame directly
--- git.c | 143 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 82 insertions(+), 61 deletions(-)
git.c +82 -61
@@ -5,17 +5,17 @@#include "run-command.h" #include "alias.h" -#define RUN_SETUP (1<<0) -#define RUN_SETUP_GENTLY (1<<1) -#define USE_PAGER (1<<2) +#define RUN_SETUP (1 << 0) +#define RUN_SETUP_GENTLY (1 << 1) +#define USE_PAGER (1 << 2)/* * require working tree to be present -- anything uses this needs * RUN_SETUP for reading from the configuration file. */ -#define NEED_WORK_TREE (1<<3) -#define SUPPORT_SUPER_PREFIX (1<<4) -#define DELAY_PAGER_CONFIG (1<<5) -#define NO_PARSEOPT (1<<6)/* parse-options is not used */ +#define NEED_WORK_TREE (1 << 3) +#define SUPPORT_SUPER_PREFIX (1 << 4) +#define DELAY_PAGER_CONFIG (1 << 5) +#define NO_PARSEOPT (1 << 6)/* parse-options is not used */ struct cmd_struct {const char *cmd;
@@ -23,22 +23,23 @@ struct cmd_struct {unsigned int option; }; -const char git_usage_string[] = -N_ ("git [--version] [--help] [-C <path>] [-c <name>=<value>]\n" -" [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n" -" [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--bare]\n" -" [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]\n" -" <command> [<args>]" ); +const char git_usage_string[] =N_ ( +"git [--version] [--help] [-C <path>] [-c <name>=<value>]\n" +" [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n" +" [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--bare]\n" +" [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]\n" +" <command> [<args>]" ); -const char git_more_info_string[] = -N_ ("'git help -a' and 'git help -g' list available subcommands and some\n" -"concept guides. See 'git help <command>' or 'git help <concept>'\n" -"to read about a specific subcommand or concept.\n" -"See 'git help git' for an overview of the system." ); +const char git_more_info_string[] =N_ ( +"'git help -a' and 'git help -g' list available subcommands and some\n" +"concept guides. See 'git help <command>' or 'git help <concept>'\n" +"to read about a specific subcommand or concept.\n" +"See 'git help git' for an overview of the system." );static int use_pager =-1 ; -static void list_builtins (struct string_list *list,unsigned int exclude_option); +static void list_builtins (struct string_list *list, +unsigned int exclude_option);static void exclude_helpers_from_list (struct string_list *list) {
@@ -66,9 +67,9 @@ static int list_cmds(const char *spec)int nongit;/* -* Set up the repository so we can pick up any repo-level config (like -* completion.commands). -*/ +* Set up the repository so we can pick up any repo-level config (like +* completion.commands). +*/ setup_git_directory_gently (&nongit);while (*spec) {
@@ -93,8 +94,7 @@ static int list_cmds(const char *spec)strbuf_add (&sb, spec +5 , len -5 );list_cmds_by_category (&list, sb.buf);strbuf_release (&sb); - } -else + }else die (_ ("unsupported command listing type '%s'" ), spec); spec += len;if (*spec ==',' )
@@ -183,7 +183,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--git-dir" )) {if (*argc <2 ) { -fprintf (stderr,_ ("no directory given for --git-dir\n" )); +fprintf (stderr, +_ ("no directory given for --git-dir\n" ));usage (git_usage_string); }setenv (GIT_DIR_ENVIRONMENT , (*argv)[1 ],1 );
@@ -197,7 +198,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--namespace" )) {if (*argc <2 ) { -fprintf (stderr,_ ("no namespace given for --namespace\n" )); +fprintf (stderr, +_ ("no namespace given for --namespace\n" ));usage (git_usage_string); }setenv (GIT_NAMESPACE_ENVIRONMENT , (*argv)[1 ],1 );
@@ -211,7 +213,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--work-tree" )) {if (*argc <2 ) { -fprintf (stderr,_ ("no directory given for --work-tree\n" )); +fprintf (stderr, +_ ("no directory given for --work-tree\n" ));usage (git_usage_string); }setenv (GIT_WORK_TREE_ENVIRONMENT , (*argv)[1 ],1 );
@@ -225,7 +228,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--super-prefix" )) {if (*argc <2 ) { -fprintf (stderr,_ ("no prefix given for --super-prefix\n" )); +fprintf (stderr, +_ ("no prefix given for --super-prefix\n" ));usage (git_usage_string); }setenv (GIT_SUPER_PREFIX_ENVIRONMENT , (*argv)[1 ],1 );
@@ -247,7 +251,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"-c" )) {if (*argc <2 ) { -fprintf (stderr,_ ("-c expects a configuration string\n" )); +fprintf (stderr, +_ ("-c expects a configuration string\n" ));usage (git_usage_string); }git_config_push_parameter ((*argv)[1 ]);
@@ -280,17 +285,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) }else if (!strcmp (cmd,"--shallow-file" )) { (*argv)++; (*argc)--; -set_alternate_shallow_file (the_repository, (*argv)[0 ],1 ); +set_alternate_shallow_file (the_repository, (*argv)[0 ], +1 );if (envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"-C" )) {if (*argc <2 ) { -fprintf (stderr,_ ("no directory given for -C\n" )); +fprintf (stderr, +_ ("no directory given for -C\n" ));usage (git_usage_string); }if ((*argv)[1 ][0 ]) {if (chdir ((*argv)[1 ])) -die_errno ("cannot change to '%s'" , (*argv)[1 ]); +die_errno ("cannot change to '%s'" , + (*argv)[1 ]);if (envchanged) *envchanged =1 ; }
@@ -354,7 +362,7 @@ static int handle_alias(int *argcp, const char ***argv)trace2_cmd_name ("_run_shell_alias_" ); ret =run_command (&child); -if (ret >=0 )/* normal exit */ +if (ret >=0 )/* normal exit */ exit (ret); die_errno(_("while expanding alias '%s': '%s'"),
@@ -378,8 +386,7 @@ static int handle_alias(int *argcp, const char ***argv)if (!strcmp (alias_command, new_argv[0 ]))die (_ ("recursive alias: %s" ), alias_command); -trace_argv_printf (new_argv, -"trace: alias expansion: %s =>" , +trace_argv_printf (new_argv,"trace: alias expansion: %s =>" , alias_command);REALLOC_ARRAY (new_argv, count + *argcp);
@@ -416,14 +423,16 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) prefix =setup_git_directory_gently (&nongit_ok); } -if (use_pager ==-1 && p->option & (RUN_SETUP |RUN_SETUP_GENTLY ) && +if (use_pager ==-1 && + p->option & (RUN_SETUP |RUN_SETUP_GENTLY ) && !(p->option &DELAY_PAGER_CONFIG )) use_pager =check_pager_config (p->cmd);if (use_pager ==-1 && p->option &USE_PAGER ) use_pager =1 ;if ((p->option & (RUN_SETUP |RUN_SETUP_GENTLY )) && - startup_info->have_repository)/* get_git_dir() may set up repo, avoid that */ + startup_info->have_repository)/* get_git_dir() may set up +repo, avoid that */ trace_repo_setup (prefix); }commit_pager_choice ();
@@ -467,7 +476,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)static struct cmd_struct commands[] = { {"add" , cmd_add,RUN_SETUP |NEED_WORK_TREE }, {"am" , cmd_am,RUN_SETUP |NEED_WORK_TREE }, - {"annotate" , cmd_annotate,RUN_SETUP |NO_PARSEOPT }, + {"annotate" , cmd_blame,RUN_SETUP }, {"apply" , cmd_apply,RUN_SETUP_GENTLY }, {"archive" , cmd_archive,RUN_SETUP_GENTLY }, {"bisect--helper" , cmd_bisect__helper,RUN_SETUP },
@@ -478,10 +487,9 @@ static struct cmd_struct commands[] = { { "check-attr", cmd_check_attr, RUN_SETUP }, { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE }, { "check-mailmap", cmd_check_mailmap, RUN_SETUP }, - { "check-ref-format", cmd_check_ref_format, NO_PARSEOPT }, + { "check-ref-format", cmd_check_ref_format, NO_PARSEOPT }, { "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE }, - { "checkout-index", cmd_checkout_index, - RUN_SETUP | NEED_WORK_TREE}, + { "checkout-index", cmd_checkout_index, RUN_SETUP | NEED_WORK_TREE }, { "cherry", cmd_cherry, RUN_SETUP }, { "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE }, { "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
@@ -495,7 +503,8 @@ static struct cmd_struct commands[] = { { "credential", cmd_credential, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "describe", cmd_describe, RUN_SETUP }, { "diff", cmd_diff, NO_PARSEOPT }, - { "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, + { "diff-files", cmd_diff_files, + RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, { "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT }, { "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT }, { "difftool", cmd_difftool, RUN_SETUP_GENTLY },
@@ -528,10 +537,14 @@ static struct cmd_struct commands[] = { { "merge-file", cmd_merge_file, RUN_SETUP_GENTLY }, { "merge-index", cmd_merge_index, RUN_SETUP | NO_PARSEOPT }, { "merge-ours", cmd_merge_ours, RUN_SETUP | NO_PARSEOPT }, - { "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, - { "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, - { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, - { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, + { "merge-recursive", cmd_merge_recursive, + RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, + { "merge-recursive-ours", cmd_merge_recursive, + RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, + { "merge-recursive-theirs", cmd_merge_recursive, + RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, + { "merge-subtree", cmd_merge_recursive, + RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, { "merge-tree", cmd_merge_tree, RUN_SETUP | NO_PARSEOPT }, { "mktag", cmd_mktag, RUN_SETUP | NO_PARSEOPT }, { "mktree", cmd_mktree, RUN_SETUP },
@@ -549,9 +562,10 @@ static struct cmd_struct commands[] = { { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE }, { "push", cmd_push, RUN_SETUP }, { "range-diff", cmd_range_diff, RUN_SETUP | USE_PAGER }, - { "read-tree", cmd_read_tree, RUN_SETUP | SUPPORT_SUPER_PREFIX}, + { "read-tree", cmd_read_tree, RUN_SETUP | SUPPORT_SUPER_PREFIX }, { "rebase", cmd_rebase, RUN_SETUP | NEED_WORK_TREE }, - { "rebase--interactive", cmd_rebase__interactive, RUN_SETUP | NEED_WORK_TREE }, + { "rebase--interactive", cmd_rebase__interactive, + RUN_SETUP | NEED_WORK_TREE }, { "receive-pack", cmd_receive_pack }, { "reflog", cmd_reflog, RUN_SETUP }, { "remote", cmd_remote, RUN_SETUP },
@@ -581,7 +595,8 @@ static struct cmd_struct commands[] = { { "stash", cmd_stash }, { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE }, { "stripspace", cmd_stripspace }, - { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT }, + { "submodule--helper", cmd_submodule__helper, + RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT }, { "switch", cmd_switch, RUN_SETUP | NEED_WORK_TREE }, { "symbolic-ref", cmd_symbolic_ref, RUN_SETUP }, { "tag", cmd_tag, RUN_SETUP | DELAY_PAGER_CONFIG },
@@ -623,8 +638,7 @@ static void list_builtins(struct string_list *out, unsigned int exclude_option) {int i;for (i =0 ; i <ARRAY_SIZE (commands); i++) { -if (exclude_option && - (commands[i].option & exclude_option)) +if (exclude_option && (commands[i].option & exclude_option))continue ;string_list_append (out, commands[i].cmd); }
@@ -752,7 +766,8 @@ static int run_argv(int *argcp, const char ***argv)trace2_cmd_name ("_run_git_alias_" );if (get_super_prefix ()) -die ("%s doesn't support --super-prefix" , **argv); +die ("%s doesn't support --super-prefix" , + **argv);commit_pager_choice ();
@@ -764,10 +779,13 @@ static int run_argv(int *argcp, const char ***argv)/* * if we fail because the command is not found, it is -* OK to return. Otherwise, we just pass along the status code. +* OK to return. Otherwise, we just pass along the +* status code. */ - i =run_command_v_opt_tr2 (args.argv,RUN_SILENT_EXEC_FAILURE | -RUN_CLEAN_ON_EXIT ,"git_alias" ); + i =run_command_v_opt_tr2 (args.argv, +RUN_SILENT_EXEC_FAILURE | +RUN_CLEAN_ON_EXIT , +"git_alias" );if (i >=0 || errno !=ENOENT )exit (i);die ("could not execute builtin %s" , **argv);
@@ -781,7 +799,8 @@ static int run_argv(int *argcp, const char ***argv)int i;struct strbuf sb =STRBUF_INIT ;for (i =0 ; i < cmd_list.nr; i++) { -struct string_list_item *item = &cmd_list.items[i]; +struct string_list_item *item = + &cmd_list.items[i];strbuf_addf (&sb,"\n %s" , item->string);if (item == seen)
@@ -790,7 +809,8 @@ static int run_argv(int *argcp, const char ***argv)strbuf_addstr (&sb," ==>" ); }die (_ ("alias loop detected: expansion of '%s' does" -" not terminate:%s" ), cmd_list.items[0 ].string, sb.buf); +" not terminate:%s" ), + cmd_list.items[0 ].string, sb.buf); }string_list_append (&cmd_list, *argv[0 ]);
@@ -872,8 +892,9 @@ int cmd_main(int argc, const char **argv)if (errno !=ENOENT )break ;if (was_alias) { -fprintf (stderr,_ ("expansion of alias '%s' failed; " -"'%s' is not a git command\n" ), +fprintf (stderr, +_ ("expansion of alias '%s' failed; " +"'%s' is not a git command\n" ), cmd, argv[0 ]);exit (1 ); }
@@ -884,8 +905,8 @@ int cmd_main(int argc, const char **argv)break ; } -fprintf (stderr,_ ("failed to run command '%s': %s\n" ), - cmd,strerror (errno)); +fprintf (stderr,_ ("failed to run command '%s': %s\n" ), cmd, +strerror (errno));return 1 ; }
Signature · 1 line
gitgitgadget
PATCH 2/5 edit the makefile
In reply to Abimbola Olaitan via GitGitGadgetFrom: Abimbola <[email protected]>
--- Makefile | 1 - 1 file changed, 1 deletion(-)
Makefile +0 -1
@@ -1033,7 +1033,6 @@ LIB_OBJS += zlib.o BUILTIN_OBJS += builtin/add.o BUILTIN_OBJS += builtin/am.o -BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o BUILTIN_OBJS += builtin/bisect--helper.o
Signature · 1 line
gitgitgadget
PATCH 3/5 remove-annotate: comment out annotate BUILDIN_OBJS
In reply to Abimbola Olaitan via GitGitGadgetFrom: Abimbola <[email protected]>
--- Makefile | 1 + 1 file changed, 1 insertion(+)
Makefile +1 -0
@@ -1033,6 +1033,7 @@ LIB_OBJS += zlib.o BUILTIN_OBJS += builtin/add.o BUILTIN_OBJS += builtin/am.o +// BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o BUILTIN_OBJS += builtin/bisect--helper.o
Signature · 1 line
gitgitgadget
PATCH 4/5 remove-annotate: delete builtin annotate file
In reply to Abimbola Olaitan via GitGitGadgetFrom: Abimbola <[email protected]>
--- annotate.txt | Bin 0 -> 163170 bytes blame.txt | Bin 0 -> 177882 bytes builtin/annotate.c | 22 ---------------------- 3 files changed, 22 deletions(-) create mode 100644 annotate.txt create mode 100644 blame.txt delete mode 100644 builtin/annotate.c
annotate.txt new file binary +0 -0
blame.txt new file binary +0 -0
builtin/annotate.c deleted +0 -22
@@ -1,22 +0,0 @@ -/* -* "git annotate" builtin alias -* -* Copyright (C) 2006 Ryan Anderson -*/ -#include "git-compat-util.h" -#include "builtin.h" -#include "argv-array.h" - -int cmd_annotate (int argc,const char **argv,const char *prefix) -{ -struct argv_array args =ARGV_ARRAY_INIT ; -int i; - -argv_array_pushl (&args,"annotate" ,"-c" ,NULL ); - -for (i =1 ; i < argc; i++) { -argv_array_push (&args, argv[i]); - } - -return cmd_blame (args.argc, args.argv, prefix); -}
Signature · 1 line
gitgitgadget
PATCH 5/5 fix unproper comment in Makefile
In reply to Abimbola Olaitan via GitGitGadgetFrom: Abimbola <[email protected]>
--- Makefile | 1 - 1 file changed, 1 deletion(-)
Makefile +0 -1
@@ -1033,7 +1033,6 @@ LIB_OBJS += zlib.o BUILTIN_OBJS += builtin/add.o BUILTIN_OBJS += builtin/am.o -// BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o BUILTIN_OBJS += builtin/bisect--helper.o
Signature · 1 line
gitgitgadget
Re: [PATCH 1/5] remove-annotate: change cmd_annotate to cmd_blame
In reply to Abimbola via GitGitGadgetQuoted 5 lines · Abimbola via GitGitGadget
Subject: Re: [PATCH 1/5] remove-annotate: change
cmd_annotatetocmd_blame
From: Abimbola <[email protected]>Changing this command is to remove the
annotate.cfile which does almost the same thing asblame.c. git annotate will invoke blame directly
Both the summary and the detailed description of the changes is not entirely clean. What is this 'remove-annotate' subsystem? Why we would want to change cmd_annotate to cmd_blame -- and wouldn't we loose difference (in defaults, and thus in output format) between git-blame and git-annotate?
Quoted 2 lines
Signed-off-by Abimbola <[email protected]>
Why not
Quoted 3 lines
--- git.c | 143 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 82 insertions(+), 61 deletions(-)
Looks like a big change, strangely. And annotate.c is not actually deleted...
Quoted 6 lines
git.c +0
-0
git.c line 10
Quoted 9 lines
git.c +3
-3@@ -5,17 +5,17 @@#include "run-command.h" #include "alias.h" -#define RUN_SETUP (1<<0) -#define RUN_SETUP_GENTLY (1<<1) -#define USE_PAGER (1<<2) +#define RUN_SETUP (1 << 0) +#define RUN_SETUP_GENTLY (1 << 1) +#define USE_PAGER (1 << 2)
Please, please do not include such unnecessary (and possibly accidental) changes.
It makes review harded (where are the actual changes), and it makes merging it harder (as it can much more easily conflict with other changes in flight).
Also, those changes that remove aligning of values make code less readable, and would probably be rejected.
[skip whitespace-only changes (I hope)]
git.c struct cmd_struct { · line 42
Quoted 3 lines
git.c +2
-1-static void list_builtins (struct string_list *list,unsigned int exclude_option); +static void list_builtins (struct string_list *list, +unsigned int exclude_option);
This may be a good cleanup, but is unrelated to the change in question. Better leave it to a separate patch.
[skip horizontal and vertical whitespace changes (I hope)]
git.c static int list_cmds(const char *spec) · line 100
Quoted 10 lines
git.c +1
-2@@ -93,8 +94,7 @@ static int list_cmds(const char *spec)strbuf_add (&sb, spec +5 , len -5 );list_cmds_by_category (&list, sb.buf);strbuf_release (&sb); - } -else + }else die (_ ("unsupported command listing type '%s'" ), spec); spec += len;if (*spec ==',' )
This may be a good cleanup, but is unrelated to the change in question, Better leave it to another separate patch.
git.c static int run_builtin(struct cmd_struct *p, int argc, const char **argv) · line 482
Quoted 9 lines
git.c +1
-1@@ -467,7 +476,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)static struct cmd_struct commands[] = { {"add" , cmd_add,RUN_SETUP |NEED_WORK_TREE }, {"am" , cmd_am,RUN_SETUP |NEED_WORK_TREE }, - {"annotate" , cmd_annotate,RUN_SETUP |NO_PARSEOPT }, + {"annotate" , cmd_blame,RUN_SETUP }, {"apply" , cmd_apply,RUN_SETUP_GENTLY }, {"archive" , cmd_archive,RUN_SETUP_GENTLY }, {"bisect--helper" , cmd_bisect__helper,RUN_SETUP },
That looks like the major portion of the change. How the look and feel of git-annotate command is preserved, though?
git.c static struct cmd_struct commands[] = { · line 490
Quoted 6 lines
git.c +1
-1@@ -478,10 +487,9 @@ static struct cmd_struct commands[] = { { "check-attr", cmd_check_attr, RUN_SETUP }, { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE }, { "check-mailmap", cmd_check_mailmap, RUN_SETUP }, - { "check-ref-format", cmd_check_ref_format, NO_PARSEOPT }, + { "check-ref-format", cmd_check_ref_format, NO_PARSEOPT },
This may be a good cleanup, but is unrelated to the change in question. Better leave it to a patch.
Regards,
Signature · 1 line
Jakub Narębski
Re: [PATCH 1/5] remove-annotate: change cmd_annotate to cmd_blame
Quoted 19 lines · Jakub Narebski
Quoted 5 lines · Abimbola via GitGitGadget
Subject: Re: [PATCH 1/5] remove-annotate: change
cmd_annotatetocmd_blame
From: Abimbola <[email protected]>Changing this command is to remove the
annotate.cfile which does almost the same thing asblame.c. git annotate will invoke blame directlyBoth the summary and the detailed description of the changes is not entirely clean. What is this 'remove-annotate' subsystem? Why we would want to change
cmd_annotatetocmd_blame-- and wouldn't we loose difference (in defaults, and thus in output format) between git-blame and git-annotate?Quoted 2 lines
Signed-off-by Abimbola <[email protected]>Why not
Quoted 8 lines
Signed-off-by Abimbola Olaitan <[email protected]>Quoted 3 lines
--- git.c | 143 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 82 insertions(+), 61 deletions(-)Looks like a big change, strangely. And
annotate.cis not actually deleted...
Thanks for all the good comments. The first priority of any work in this area is not to lose the different defaults between "annotate" and "blame", as the former has ("had", at least when we invented "blame") known uses in widely used script(s), and the solution we took was to use a small annotate.c shim to tell the blame machinery to emit historical "annotate" output format. If we can do so without having the shim, that would be OK. If removal of shim loses the distinction, then it is not.
I suspect that cmd_blame() can be taught to look at argv[0] and pretend "-c" was given on the command line, and then the command dispatch table can call cmd_blame() upon seeing "annotate" as the subcommand name, but I haven't actually tried it so there may be hidden dragons.
Thanks.
Re: [PATCH 5/5] fix unproper comment in Makefile
In reply to Abimbola via GitGitGadgetQuoted 12 lines · Abimbola via GitGitGadget
From: Abimbola <[email protected]>
Signed-off-by Abimbola <[email protected]>--- Makefile | 1 - 1 file changed, 1 deletion(-)Makefile +0
-0
Makefile LIB_OBJS += zlib.o · line 1038
Quoted 7 lines
Makefile +0
-1@@ -1033,7 +1033,6 @@ LIB_OBJS += zlib.o BUILTIN_OBJS += builtin/add.o BUILTIN_OBJS += builtin/am.o -// BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o BUILTIN_OBJS += builtin/bisect--helper.o
If you are ever going to reroll this topic, please clean up your series before sending it out. 3/5 introduced a bogus line this step removes; it should have used the right comment char "#" instead, or better yet, just drop both.
Find where you are doing "oops, the change done in the previous step was wrong, and here is a commit to bring it into the right state" and learn to use "rebase -i" to eliminate such a "oops, another band aid on top"; it is a useful skill.
Having said that, if the topic is only to remove builtin/annotate.c, I would expect that it would be a single patch topic (i.e. Makefile, reference in git.c and builtin/annotate.c all must be modified or removed at the same time).
Thanks.