[PATCH] simplifying branch.c:install_branch_config() if()
Signed-off-by: Nemina Amarasinghe nemi...@gmail.com --- branch.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/branch.c b/branch.c index 723a36b..fd93603 100644 --- a/branch.c +++ b/branch.c @@ -87,16 +87,11 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(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) + else if (!remote_is_branch) 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); -- 1.9.0.152.g6ab4ae2 -- 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] simplifying branch.c:install_branch_config() if()
Is this safe? Today branch.c::create_branch checks that the argument to e.g. --set-upstream-to is either in refs/heads/* or the image of some remote, but what is making sure that remains true tomorrow? So if changing this, I would be happier if the if condition were still (!remote_is_branch origin) so the impossible case could emit a BUG. Hope that helps, Jonathan Thanks for the coments Jonathan, Yes you are correct... I was just thought about how to simplify this chain of if() statement. Not the big picture. Now I understand when I change or implenet something I have to think not only about the current matter but abot the future also. Nemina -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
Eric Sunshine sunshine at sunshineco.com writes:
On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe neminaa at
gmail.com wrote:
Nemina Amarasinghe neminaa at gmail.com writes:
Sorry for the first patch. Something went wrong. This is the correct one
In addition to the tautological issue pointed out by Matthieu, please
note that this version of the patch is not the correct one. It won't
apply to the git code. At a guess, it appears that this patch is
against some other modification you made to the git code before this
change, or perhaps you committed it incorrectly. In any event, when
you resubmit, please be sure that the new version can be applied to
commit.c as it exists in git.git itself.
Thank you very much for your comments Eric. I will resubmit the patch.
Just a quick question
in this code
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 remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
else
die(BUG: impossible combination of %d and %p,
remote_is_branch, origin);
}
These local and remote variables are independent from the origin right?
So, If that the case couldn't we just use the bellow function
else if (!remote_is_branch)
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);
instead of
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 remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
Thanks
Nemina
--
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][GSoC]simplified branch.c:install_branch_config() if() statement
I hope this is the correct format for patch. Please comment on this if something is wrong. Signed-off-by:Nemina Amarasinghe nemi...@gmail.com --- branch.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/branch.c b/branch.c index 0304a7a..fd93603 100644 --- a/branch.c +++ b/branch.c @@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(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) + else if (!remote_is_branch) printf_ln(rebasing ? _(Branch %s set up to track remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), -- 1.9.0.152.g6ab4ae2 -- 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
[PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Did some simple changing to the chain of if() statements in function install_branch_config() of branch.c This was a micro-project for GSOC 2014 From aebfa60feb643280c89b54e5ab87f9d960cde452 Mon Sep 17 00:00:00 2001 From: Nemina Amarasinghe nemi...@gmail.com Date: Mon, 10 Mar 2014 13:02:55 +0530 Subject: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c --- branch.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/branch.c b/branch.c index d3b9d49..0304a7a 100644 --- a/branch.c +++ b/branch.c @@ -87,7 +87,12 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(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 || !origin)) + 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 remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), -- 1.9.0.152.g6ab4ae2 -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch. Something went wrong. This is the correct one From aebfa60feb643280c89b54e5ab87f9d960cde452 Mon Sep 17 00:00:00 2001 From: Nemina Amarasinghe nemi...@gmail.com Date: Mon, 10 Mar 2014 13:02:55 +0530 Subject: [PATCH] simplified the chain if() statements of install_brach_config() function in branch.c --- branch.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/branch.c b/branch.c index d3b9d49..0304a7a 100644 --- a/branch.c +++ b/branch.c @@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(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) + else if (!remote_is_branch (origin || !origin)) printf_ln(rebasing ? _(Branch %s set up to track remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), -- 1.9.0.152.g6ab4ae2 -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe neminaa at gmail.com writes: Is it me, or is (origin || !origin) a tautology? Thanks for the advices Matthieu. I will go through the documentations again. Is there anything wrong with my logic? What I wanted to express is ((!remote_is_branch origin) || (!remote_is_branch || !origin)) -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
((!remote_is_branch origin) || (!remote_is_branch || !origin)) Is it? The above is the same as (!remote_is_branch || !origin). What you wrote before is the same as (!remote_is_branch). Maybe you should try copypaste from the expressions you are trying to combine to make sure that what you start with makes sense. OMG.. Really sorry for that... that was a silly mistake. This is the one.. ((!remote_is_branch origin) || (!remote_is_branch !origin)) -- 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
