* [PATCH] install_branch_config(): switch from 'else-if' series to 'nested if-else'
@ 2014-03-12 8:34 Nishhal Gaba
2014-03-12 9:23 ` Matthieu Moy
0 siblings, 1 reply; 2+ messages in thread
From: Nishhal Gaba @ 2014-03-12 8:34 UTC (permalink / raw)
To: git; +Cc: Nishchal
From: Nishchal <nishgaba9@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 <nishgaba9@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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] install_branch_config(): switch from 'else-if' series to 'nested if-else'
2014-03-12 8:34 [PATCH] install_branch_config(): switch from 'else-if' series to 'nested if-else' Nishhal Gaba
@ 2014-03-12 9:23 ` Matthieu Moy
0 siblings, 0 replies; 2+ messages in thread
From: Matthieu Moy @ 2014-03-12 9:23 UTC (permalink / raw)
To: Nishhal Gaba; +Cc: git
Nishhal Gaba <nishgaba9@gmail.com> writes:
> From: Nishchal <nishgaba9@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/
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-03-12 9:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-12 8:34 [PATCH] install_branch_config(): switch from 'else-if' series to 'nested if-else' Nishhal Gaba
2014-03-12 9:23 ` Matthieu Moy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).