On Mon, Mar 19, 2012 at 11:58:02AM -0700, Junio C Hamano wrote:
> Christopher Tiwald <christiwald@xxxxxxxxx> writes:
>
> > Pushing a non-fast-forward update to a remote repository will result in
> > an error, but the hint text doesn't provide the correct resolution in
> > every case. Give better resolution advice in three push scenarios:
> >
> > 1) If you push a non-fast-forward update to your current branch, you
> > should merge remote changes with 'git pull' before pushing again.
>
> I have always found "update *to* your current branch" very strange
> phrasing (the earlier one said "to HEAD", but it amounts to the same
> thing). You do not push *to* your branch. You push your branch to
> somewhere else (namely, remote). I would understand if it said "If your
> push of your current branch triggers a non-ff error, ...", though.
Ah. Yeah. I can see the problem with my phrasing now. How about something
like the following?
"If you push your current branch and it triggers a non-fast-forward
error, you should merge remote changes with 'git pull' before pushing
again."
> She never gets a chance to see the other checkout-pull-push message, does
> she?
>
> > There is one aspect about this patch about which I'm unsure: What to
> > do with users who've set "advice.pushNonFastForward = false" already.
>
> The change in this patch is merely clarifying what pushNonFastForward
> advise has already taught them ("Non-ff was rejected; the manual will tell
> you what you wanted to do") by dividing them into three categories and
> giving different advices to these categories. As the user says he
> understood what he is doing, I think squelching all of them is a sane
> choice.
How about the something like the following fixup? This introduces two
changes to v2:
- It breaks the new advice into three config variables. Users
who might benefit from the advice can't accidentally shut a message
off before being confronted with the situation it's designed to
advise.
- It leaves pushNonFastForward in place, and if a user sets
'advice.pushNonFastForward = false', it'll disable all three pieces
of advice.
--
Christopher Tiwald
--- 8< ---
Signed-off-by: Christopher Tiwald <christiwald@xxxxxxxxx>
---
Documentation/config.txt | 19 +++++++++++++++----
advice.c | 8 ++++++--
advice.h | 4 +++-
builtin/push.c | 6 +++---
4 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a2329b5..fb386ab 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -137,13 +137,24 @@ advice.*::
can tell Git that you do not need help by setting these to 'false':
+
--
+ pushNonFastForward::
+ Set this variable to 'false' if you want to disable
+ 'pushNonFFCurrent', 'pushNonFFDefault', and
+ 'pushNonFFMatching' simultaneously.
pushNonFFCurrent::
Advice shown when linkgit:git-push[1] fails due to a
non-fast-forward update to the current branch.
- pushNonFFOther::
- Advice shown when linkgit:git-push[1] fails due to a
- non-fast-forward update to a branch other than the
- current one.
+ pushNonFFDefault::
+ Advice to set 'push.default' to 'upstream' or 'current'
+ when you ran linkgit:git-push[1] and pushed 'matching
+ refs' by default (i.e. you did not provide an explicit
+ refspec, and no 'push.default' configuration was set)
+ and it resulted in a non-fast-forward error.
+ pushNonFFMatching::
+ Advice shown when you ran linkgit:git-push[1] and pushed
+ 'matching refs' explicitly (i.e. you used ':', or
+ specified a refspec that isn't your current branch) and
+ it resulted in a non-fast-forward error.
statusHints::
Directions on how to stage/unstage/add shown in the
output of linkgit:git-status[1] and the template shown
diff --git a/advice.c b/advice.c
index ee62e1b..a492eea 100644
--- a/advice.c
+++ b/advice.c
@@ -1,7 +1,9 @@
#include "cache.h"
+int advice_push_nonfastforward = 1;
int advice_push_non_ff_current = 1;
-int advice_push_non_ff_other = 1;
+int advice_push_non_ff_default = 1;
+int advice_push_non_ff_matching = 1;
int advice_status_hints = 1;
int advice_commit_before_merge = 1;
int advice_resolve_conflict = 1;
@@ -12,8 +14,10 @@ static struct {
const char *name;
int *preference;
} advice_config[] = {
+ { "pushnonfastforward", &advice_push_nonfastforward },
{ "pushnonffcurrent", &advice_push_non_ff_current },
- { "pushnonffother", &advice_push_non_ff_other },
+ { "pushnonffdefault", &advice_push_non_ff_default },
+ { "pushnonffmatching", &advice_push_non_ff_matching },
{ "statushints", &advice_status_hints },
{ "commitbeforemerge", &advice_commit_before_merge },
{ "resolveconflict", &advice_resolve_conflict },
diff --git a/advice.h b/advice.h
index 98c675e..f3cdbbf 100644
--- a/advice.h
+++ b/advice.h
@@ -3,8 +3,10 @@
#include "git-compat-util.h"
+extern int advice_push_nonfastforward;
extern int advice_push_non_ff_current;
-extern int advice_push_non_ff_other;
+extern int advice_push_non_ff_default;
+extern int advice_push_non_ff_matching;
extern int advice_status_hints;
extern int advice_commit_before_merge;
extern int advice_resolve_conflict;
diff --git a/builtin/push.c b/builtin/push.c
index 3de2737..a0ffbb3 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -138,21 +138,21 @@ static const char message_advice_checkout_pull_push[] =
static void advise_pull_before_push(void)
{
- if (!advice_push_non_ff_current)
+ if (!advice_push_non_ff_current | !advice_push_nonfastforward)
return;
advise(_(message_advice_pull_before_push));
}
static void advise_use_upstream(void)
{
- if (!advice_push_non_ff_other)
+ if (!advice_push_non_ff_default | !advice_push_nonfastforward)
return;
advise(_(message_advice_use_upstream));
}
static void advise_checkout_pull_push(void)
{
- if (!advice_push_non_ff_other)
+ if (!advice_push_non_ff_matching | !advice_push_nonfastforward)
return;
advise(_(message_advice_checkout_pull_push));
}
--
1.7.10.rc1.23.g2a051.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html