Re: [PATCH/RFC] builtin: move builtin retrieval to get_builtin()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 13, 2014, 18:19 UTC
- Message-ID
- <xmqq389n2cac.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <5463DA20.3080703@inventati.org>
slavomir vlcek <svlc@inventati.org> writes:
Show 9 quoted lines
> 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 <svlc@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...
Show 11 quoted lines
> From 78228e3f7c3029d07827f973fa7992777d6e0cb9 Mon Sep 17 00:00:00 2001 > From: slavomir vlcek <svlc@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.
Show 13 quoted lines
> 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.
Show 10 quoted lines
> @@ -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.
Show 8 quoted lines
> + 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?
> } > }