slavomir vlcek <s...@inventati.org> writes:
> Hi,
> found a small code redundancy in a builtin command retrieval ('git.c').
>
> For the "master" branch.
>
> Thanks in advance for any suggestions.
>
> Signed-off-by: slavomir vlcek <s...@inventati.org>
> ---
Thanks. Please do realize that all of the above before the
three-dash line and nothing else will be made into the commit log
message (together with what you wrote on the Subject: line).
Which means these lines...
> From 78228e3f7c3029d07827f973fa7992777d6e0cb9 Mon Sep 17 00:00:00 2001
> From: slavomir vlcek <s...@inventati.org>
> Date: Wed, 12 Nov 2014 14:10:22 +0100
> Subject: [PATCH] builtin: move builtin retrieval to get_builtin()
>
> There was a redundant code for a builtin command retrieval
> in 'handle_builtin()' and 'is_builtin()'.
>
> This was solved by adding a new function 'get_builtin()'
> and by making a boolean wrapper out of the 'is_builtin()'.
> ---
... will not be part of the log message, which is definitely wrong.
To correct this:
$ git checkout 78228e3f7c3029d0
$ git commit --amend -s --no-edit
to add your sign-off in the log message, then do
$ git format-patch -1 --stdout >patch.mail
Slurp patch.mail into your MUA, move the content on "Subject: " to
where your MUA expects to see the subject line, remove other header
material starting from "From 7822..." so that the message body
begins with "There was a redundant code for...". And send it out.
> git.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/git.c b/git.c
> index 18fbf79..e32c5b8 100644
> --- a/git.c
> +++ b/git.c
> @@ -487,15 +487,20 @@ static struct cmd_struct commands[] = {
> { "write-tree", cmd_write_tree, RUN_SETUP },
> };
>
> -int is_builtin(const char *s)
> +struct cmd_struct *get_builtin(const char *s)
I do not think this has to be extern.
static struct cmd_struct *get_builtin(const char *s)
perhaps.
> @@ -519,15 +525,12 @@ static void handle_builtin(int argc, const char **argv)
> argv[0] = cmd = "help";
> }
>
> - for (i = 0; i < ARRAY_SIZE(commands); i++) {
> - struct cmd_struct *p = commands+i;
> - if (strcmp(p->cmd, cmd))
> - continue;
> - if (saved_environment && (p->option & NO_SETUP)) {
> + builtin = get_builtin(cmd);
Nice.
> + if (builtin) {
> + if (saved_environment && (builtin->option & NO_SETUP))
> restore_env();
> - break;
> - }
> - exit(run_builtin(p, argc, argv));
> + else
> + exit(run_builtin(builtin, argc, argv));
This change does not seem to have anything to do with the topic of
the change. Why is it necessary?
> }
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
