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] simplified the chain if() statements of install_branch_config() function in branch.c
On Tue, Mar 11, 2014 at 2:30 AM, Nemina Amarasinghe nemi...@gmail.com wrote:
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);
}
This code does not exist in git.git. Somehow, the code you are
consulting has incorrectly substituted 'remote' for 'local' in the
strings of the (!remote_is_branch !origin) case. Whether this is
due to a local modification you made or some some other reason, it is
leading you astray in your proposed changes and breaking your patches
so that they cannot be applied to git.git.
Probably easiest at this point would be for you to start from scratch
by making a new clone of git.git and examining branch.c as it actually
exists in the 'master' branch.
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
--
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 nemi...@gmail.com writes: Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch. Something went wrong. This is the correct one Please, re-read Documentation/SubmittingPatches. In short, don't inline patch headers and don't forget the sign-off. Subject: [PATCH] simplified the chain if() statements of install_brach_config() function in branch.c Keep the subject line short (ideally 50 characters), and avoid past tense. We usually use imperative (the patch asks the codebase to refactor itself = simplify if statements We usually prefix the subject line with the place/subsystem where the change is done = branch.c: simplify if - 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)) Is it me, or is (origin || !origin) a tautology? -- 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
Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Matthieu Moy matthieu@grenoble-inp.fr writes: Nemina Amarasinghe nemi...@gmail.com writes: Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch. Something went wrong. This is the correct one Please, re-read Documentation/SubmittingPatches. In short, don't inline patch headers and don't forget the sign-off. Subject: [PATCH] simplified the chain if() statements of install_brach_config() function in branch.c Keep the subject line short (ideally 50 characters), and avoid past tense. We usually use imperative (the patch asks the codebase to refactor itself = simplify if statements We usually prefix the subject line with the place/subsystem where the change is done = branch.c: simplify if -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)) Is it me, or is (origin || !origin) a tautology? Since it _is_ you, that question is a tautology because of its first part already. But the second one would be just as good. -- David Kastrup -- 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
Nemina Amarasinghe nemi...@gmail.com writes: 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)) (The second || should be a in this sentence) I'm not saying your rewrite is incorrect, but there's no reason to keep (origin || !origin) in a logical formula. -- 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
Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe nemi...@gmail.com writes: 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)) 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. -- David Kastrup -- 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
Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe nemi...@gmail.com writes: ((!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)) That is, indeed, perfectly equivalent to (!remote_is_branch). If you write (!remote_is_branch (origin || !origin)) then you will have people (and possibly also the compiler) loudly wondering about what you are trying to say here. The suspicion would be that either this is a result of a typo or is supposed to be an annoyingly obtuse replacement for a /* TODO: treat origin and !origin differently */ kind of comment. -- David Kastrup -- 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
On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe nemi...@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. 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 -- 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
