Re: [PATCH v4] install_branch_config: simplify verbose messages logic
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 12, 2014, 05:36 UTC
- Message-ID
- <CAPig+cTmdF06PrWboJFNFEGKLCa=3rmGrcPusSsYZxdtfSEewA@mail.gmail.com>
- In-Reply-To
- <1394584412-7735-1-git-send-email-pawlo@aleg.pl>
On Tue, Mar 11, 2014 at 8:33 PM, Paweł Wawruch <pawlo@aleg.pl> wrote:
Show 6 quoted lines
> Replace the chain of if statements with table of strings. > > Signed-off-by: Paweł Wawruch <pawlo@aleg.pl> > --- > The changes proposed by Junio C Hamano: > Improvement of indentations. Removed an unused variable.
Better, thanks. More below.
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/243502 > [2]: http://thread.gmane.org/gmane.comp.version-control.git/243849 > [3]: http://thread.gmane.org/gmane.comp.version-control.git/243865
The [n]: notation typically is used for footnotes which you reference in the running text as [n] (or sometimes [*n*], depending upon preference). If you're simply listing links to previous attempts, it would be less confusing to say:
v3: http://... v2: http://... v1: http://...
Show 60 quoted lines
> branch.c | 41 +++++++++++++++++++----------------------
> 1 file changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 723a36b..2a4b911 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -53,6 +53,20 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> int remote_is_branch = starts_with(remote, "refs/heads/");
> struct strbuf key = STRBUF_INIT;
> int rebasing = should_setup_rebase(origin);
> + const char *message[][2][2] = {{{
> + N_("Branch %s set up to track remote branch %s from %s by rebasing."),
> + N_("Branch %s set up to track remote branch %s from %s."),
> + },{
> + N_("Branch %s set up to track local branch %s by rebasing."),
> + N_("Branch %s set up to track local branch %s."),
> + }},{{
> + N_("Branch %s set up to track remote ref %s by rebasing."),
> + N_("Branch %s set up to track remote ref %s."),
> + },{
> + N_("Branch %s set up to track local ref %s by rebasing."),
> + N_("Branch %s set up to track local ref %s.")
> + }}};
> + const char *name = remote_is_branch ? remote : shortname;
>
> if (remote_is_branch
> && !strcmp(local, shortname)
> @@ -77,29 +91,12 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> strbuf_release(&key);
>
> if (flag & BRANCH_CONFIG_VERBOSE) {
> - if (remote_is_branch && origin)
> - printf_ln(rebasing ?
> - _("Branch %s set up to track remote branch %s from %s by rebasing.") :
> - _("Branch %s set up to track remote branch %s from %s."),
> - local, shortname, origin);
> - else if (remote_is_branch && !origin)
> - printf_ln(rebasing ?
> - _("Branch %s set up to track local branch %s by rebasing.") :
> - _("Branch %s set up to track local branch %s."),
> - local, shortname);
> - else if (!remote_is_branch && origin)
> - printf_ln(rebasing ?
> - _("Branch %s set up to track remote ref %s by rebasing.") :
> - _("Branch %s set up to track remote ref %s."),
> - local, remote);
> - else if (!remote_is_branch && !origin)
> - printf_ln(rebasing ?
> - _("Branch %s set up to track local ref %s by rebasing.") :
> - _("Branch %s set up to track local ref %s."),
> - local, remote);
> + if (origin && remote_is_branch)
> + printf_ln(_(message[!remote_is_branch][!origin][!rebasing]),
> + local, name, origin);
> else
> - die("BUG: impossible combination of %d and %p",
> - remote_is_branch, origin);
> + printf_ln(_(message[!remote_is_branch][!origin][!rebasing]),
> + local, name);Shouldn't this logic also be encoded in the table? After all, the point of making the code table-driven is so that such hard-coded logic can be avoided. It shouldn't be difficult to do.
The same argument also applies to computation of the 'name' variable above. It too can be pushed into the the table.
Show 5 quoted lines
> } > } > > -- > 1.8.3.2