[PATCH] install_branch_config(): switch from 'else-if' series to 'nested if-else'
From: Nishchal nishga...@gmail.com I am Nishchal Gaba, a GSOC 2014 aspirant, submitting patch in response to microproject(8) Similar Execution Time, but increased readability Alternate Solution Discarded: Table driven code using commonanilty of the statements to be printed due to _() Signed-off-by: Nishchal Gaba nishga...@gmail.com --- --- branch.c | 28 +--- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/branch.c b/branch.c index 723a36b..04e9e24 100644 --- a/branch.c +++ b/branch.c @@ -77,26 +77,32 @@ 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 ? + if (origin){ + if(remote_is_branch) + 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 ? + else + 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 ? + } + + else if (!origin){ + if(remote_is_branch) + 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 + 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); -- 1.8.1.2 -- 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
Re: [PATCH] install_branch_config(): switch from 'else-if' series to 'nested if-else'
Nishhal Gaba nishga...@gmail.com writes: From: Nishchal nishga...@gmail.com Set user.email/user.name and sendemail.from to the same address to avoid this inline From:. I am Nishchal Gaba, a GSOC 2014 aspirant, submitting patch in response to microproject(8) This part of your message is the commit message. It should justify why the change is good, but who you are is not very interesting here (think of someone running git log or git blame a year from now and going through your commit, what would he expect?). The first sentence could go below the ---. Please, wrap your messages (less than 80 characters per line). Similar Execution Time, but increased readability Why capitalize Execution Time? + if (origin){ Here and below: space before { + if(remote_is_branch) space before ( + 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 ? + else + printf_ln(rebasing ? _(Branch %s set up to track remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), At this point, it would make sense to me to factor the printf_ln call like const char *msg; if (...) msg = rebasing ? _(...) : _(...); else msg = rebasing ? _(...) : _(...); printf_ln(msg, local, shortname); (but that's very subjective) - else if (!remote_is_branch !origin) - printf_ln(rebasing ? + } + + else if (!origin){ Err, isn't this the else branch of if (origin) ? If so, why repeat !origin, and more specifically, isn't the next else branch dead code: + } + else die(BUG: impossible combination of %d and %p, remote_is_branch, origin); I mean: obviously, it has to be dead code, but it seems a bit strange to read if (x) ... else if (!x) ... else die(...) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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