Patchwork discovery: if push adds a bookmark, suggest push -B

login
register
mail settings
Submitter timeless@mozdev.org
Date Sept. 10, 2015, 3:24 a.m.
Message ID <f2bf8ab52b065e46f4fb.1441855468@waste.org>
Download mbox | patch
Permalink /patch/10454/
State Changes Requested
Headers show

Comments

timeless@mozdev.org - Sept. 10, 2015, 3:24 a.m.
# HG changeset patch
# User timeless@mozdev.org
# Date 1441854648 14400
#      Wed Sep 09 23:10:48 2015 -0400
# Node ID f2bf8ab52b065e46f4fbd4a268ad4c3bb351fee3
# Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
discovery: if push adds a bookmark, suggest push -B
Augie Fackler - Sept. 10, 2015, 1:30 p.m.
On Wed, Sep 09, 2015 at 10:24:28PM -0500, timeless@mozdev.org wrote:
> # HG changeset patch
> # User timeless@mozdev.org
> # Date 1441854648 14400
> #      Wed Sep 09 23:10:48 2015 -0400
> # Node ID f2bf8ab52b065e46f4fbd4a268ad4c3bb351fee3
> # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
> discovery: if push adds a bookmark, suggest push -B

I think I don't like this - users might just follow the suggestion and
add the divergent head, which might be the wrong thing. For example, I
have a clone of Mercurial that I do all of my work in, but that I also
use to push to clowncopter. I have a number of bookmarks not yet ready
for pushing, but this would change the tip to encourage me to do just
that without reading and understanding what I'm doing.

Does that worry make sense to anyone else?

>
> diff --git a/mercurial/discovery.py b/mercurial/discovery.py
> --- a/mercurial/discovery.py
> +++ b/mercurial/discovery.py
> @@ -375,7 +375,9 @@
>                  else:
>                      error = _("push creates new remote head %s!"
>                                ) % short(dhs[0])
> -                if unsyncedheads:
> +                if repo[dhs[0]].bookmarks():
> +                    hint = _("did you mean \"hg push -B '%s'\"?") % repo[dhs[0]].bookmarks()[0]
> +                elif unsyncedheads:
>                      hint = _("pull and merge or"
>                               " see \"hg help push\" for details about"
>                               " pushing new heads")
> diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
> --- a/tests/test-bookmarks-pushpull.t
> +++ b/tests/test-bookmarks-pushpull.t
> @@ -388,7 +388,7 @@
>    pushing to http://localhost:$HGPORT2/
>    searching for changes
>    abort: push creates new remote head c922c0139ca0 with bookmark 'Y'!
> -  (merge or see "hg help push" for details about pushing new heads)
> +  (did you mean "hg push -B 'Y'"?)
>    [255]
>    $ hg -R ../a book
>       @                         1:0d2164f0ce0d
> @@ -404,7 +404,7 @@
>    pushing to http://localhost:$HGPORT2/
>    searching for changes
>    abort: push creates new remote head c922c0139ca0 with bookmark 'Y'!
> -  (merge or see "hg help push" for details about pushing new heads)
> +  (did you mean "hg push -B 'Y'"?)
>    [255]
>    $ hg -R ../a book
>       @                         1:0d2164f0ce0d
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
timeless - Sept. 10, 2015, 8:45 p.m.
Augie Fackler wrote:
>> discovery: if push adds a bookmark, suggest push -B
>
> I think I don't like this - users might just follow the suggestion and
> add the divergent head, which might be the wrong thing. For example, I
> have a clone of Mercurial that I do all of my work in, but that I also
> use to push to clowncopter. I have a number of bookmarks not yet ready
> for pushing, but this would change the tip to encourage me to do just
> that without reading and understanding what I'm doing.
>
> Does that worry make sense to anyone else?

There are two possible paths:
1. you are missing remote heads
2. you aren't

originally i treated those cases differently. if I did that, would that help?

if you're creating a new bookmark, and you aren't missing any remote
heads, then suggesting push -B seems correct to me.

if you're creating a new bookmark, and you're missing remote heads,
then you can't technically know that the bookmark is unique, but i'd
hope that it would be treated as a conflict even if you used -B
(perhaps it isn't?)

if you're changing an existing bookmark (one in remote) which was not
at remote tip, and you aren't missing remote heads, then, I don't see
a problem with suggesting -B.

if you're changing an existing bookmark, and are missing remote heads,
then I don't think it's any different than the one for a new bookmark
and missing remote heads...
Matt Mackall - Sept. 12, 2015, 10:31 p.m.
On Thu, 2015-09-10 at 09:30 -0400, Augie Fackler wrote:
> On Wed, Sep 09, 2015 at 10:24:28PM -0500, timeless@mozdev.org wrote:
> > # HG changeset patch
> > # User timeless@mozdev.org
> > # Date 1441854648 14400
> > #      Wed Sep 09 23:10:48 2015 -0400
> > # Node ID f2bf8ab52b065e46f4fbd4a268ad4c3bb351fee3
> > # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
> > discovery: if push adds a bookmark, suggest push -B
> 
> I think I don't like this - users might just follow the suggestion and
> add the divergent head, which might be the wrong thing. For example, I
> have a clone of Mercurial that I do all of my work in, but that I also
> use to push to clowncopter. I have a number of bookmarks not yet ready
> for pushing, but this would change the tip to encourage me to do just
> that without reading and understanding what I'm doing.
> 
> Does that worry make sense to anyone else?

I think we have sufficient experience with push -f to think this
particular implementation is a bad idea. A large subset of people will
reach for the nearest hammer you wave at them, without worrying whether
it's the right answer.

Patch

diff --git a/mercurial/discovery.py b/mercurial/discovery.py
--- a/mercurial/discovery.py
+++ b/mercurial/discovery.py
@@ -375,7 +375,9 @@ 
                 else:
                     error = _("push creates new remote head %s!"
                               ) % short(dhs[0])
-                if unsyncedheads:
+                if repo[dhs[0]].bookmarks():
+                    hint = _("did you mean \"hg push -B '%s'\"?") % repo[dhs[0]].bookmarks()[0]
+                elif unsyncedheads:
                     hint = _("pull and merge or"
                              " see \"hg help push\" for details about"
                              " pushing new heads")
diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -388,7 +388,7 @@ 
   pushing to http://localhost:$HGPORT2/
   searching for changes
   abort: push creates new remote head c922c0139ca0 with bookmark 'Y'!
-  (merge or see "hg help push" for details about pushing new heads)
+  (did you mean "hg push -B 'Y'"?)
   [255]
   $ hg -R ../a book
      @                         1:0d2164f0ce0d
@@ -404,7 +404,7 @@ 
   pushing to http://localhost:$HGPORT2/
   searching for changes
   abort: push creates new remote head c922c0139ca0 with bookmark 'Y'!
-  (merge or see "hg help push" for details about pushing new heads)
+  (did you mean "hg push -B 'Y'"?)
   [255]
   $ hg -R ../a book
      @                         1:0d2164f0ce0d