Re: [PATCH] [GSOC] branch.c: install_branch_config: simplify if chain
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 10, 2014, 21:18 UTC
- Message-ID
- <CAPig+cSybcLaPbMfHfRVW47o9v7qrM9XZVyoiEPf1GocUAsAWg@mail.gmail.com>
- In-Reply-To
- <1394429521-10974-1-git-send-email-Adam@sigterm.info>
On Mon, Mar 10, 2014 at 1:32 AM, Adam <adam@sigterm.info> wrote:
> Simplify if chain in install_branch_config().
Nicely done. Whether the rewritten code is indeed simpler is probably a matter of taste, however, the submission itself is well executed.
> Signed-off-by: Adam <Adam@sigterm.info>
On this project, use your full name if possible.
> ---
The GSoC microproject which begat this submission asks if it would make sense to change the code to be table-driven. If you considered this suggestion, this area just below the "---" line following your sign-off would be the perfect place to explain your reasons for rejecting it.
Show 62 quoted lines
> branch.c | 46 +++++++++++++++++++++++-----------------------
> 1 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 723a36b..b2d59f1 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -77,29 +77,29 @@ 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);
> - else
> - die("BUG: impossible combination of %d and %p",
> - remote_is_branch, origin);
> + if (remote_is_branch) {
> + if (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
> + 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 (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
> + 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);
> + }
> }
> }
>
> --
> 1.7.1