Patchwork update: when deactivating a bookmark, print a message

login
register
mail settings
Submitter Siddharth Agarwal
Date May 14, 2014, 7:50 p.m.
Message ID <8ec9ebbd09addc74ae17.1400097022@dev1738.prn1.facebook.com>
Download mbox | patch
Permalink /patch/4741/
State Superseded
Headers show

Comments

Siddharth Agarwal - May 14, 2014, 7:50 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1400096995 25200
#      Wed May 14 12:49:55 2014 -0700
# Node ID 8ec9ebbd09addc74ae17cc5f2ea13cb4c732a286
# Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
update: when deactivating a bookmark, print a message

This helps prevent user confusion when innocent-seeming commands like
'hg update -C .' are run.
Pierre-Yves David - May 15, 2014, 6:22 a.m.
On 05/14/2014 12:50 PM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1400096995 25200
> #      Wed May 14 12:49:55 2014 -0700
> # Node ID 8ec9ebbd09addc74ae17cc5f2ea13cb4c732a286
> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
> update: when deactivating a bookmark, print a message
>
> This helps prevent user confusion when innocent-seeming commands like
> 'hg update -C .' are run.

I love the idea. I always got bitten by bookmark activation or 
desactivation as a side effect of command. This should help a bit on 
this topic.

I would even encourage to add more of this when we automatically enable 
it too.

(Have not looked at the actual patch yet)
Stephen Lee - May 15, 2014, 12:50 p.m.
On 15 May 2014 16:22, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org>
wrote:
>
>
>
> On 05/14/2014 12:50 PM, Siddharth Agarwal wrote:
>>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1400096995 25200
>> #      Wed May 14 12:49:55 2014 -0700
>> # Node ID 8ec9ebbd09addc74ae17cc5f2ea13cb4c732a286
>> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
>> update: when deactivating a bookmark, print a message
>>
>> This helps prevent user confusion when innocent-seeming commands like
>> 'hg update -C .' are run.
>
>
> I love the idea. I always got bitten by bookmark activation or
desactivation as a side effect of command. This should help a bit on this
topic.
>
> I would even encourage to add more of this when we automatically enable
it too.

I sent a patch to do this on March 11.
If it won't apply anymore  I can resend tomorrow.

Steve
Matt Mackall - May 15, 2014, 4:40 p.m.
On Wed, 2014-05-14 at 12:50 -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1400096995 25200
> #      Wed May 14 12:49:55 2014 -0700
> # Node ID 8ec9ebbd09addc74ae17cc5f2ea13cb4c732a286
> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
> update: when deactivating a bookmark, print a message
> 
> This helps prevent user confusion when innocent-seeming commands like
> 'hg update -C .' are run.

Ok, your message is:

  marking bookmark c inactive

This is slightly weird to me as "active" is not so much a state of the
bookmark (ie a bit on the bookmark that allows you to mark some set of
bookmarks active/inactive) but a state of a repository (bookmark x | nil
is active). I think I might prefer:

  deactivating bookmark c

Or even:

  leaving bookmark c

..which gives more of a sense of 'why'. We might even want to add
parentheses as well to emphasize that all is normal.
Siddharth Agarwal - May 15, 2014, 4:44 p.m.
On 05/15/2014 09:40 AM, Matt Mackall wrote:
> On Wed, 2014-05-14 at 12:50 -0700, Siddharth Agarwal wrote:
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1400096995 25200
>> #      Wed May 14 12:49:55 2014 -0700
>> # Node ID 8ec9ebbd09addc74ae17cc5f2ea13cb4c732a286
>> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
>> update: when deactivating a bookmark, print a message
>>
>> This helps prevent user confusion when innocent-seeming commands like
>> 'hg update -C .' are run.
> Ok, your message is:
>
>    marking bookmark c inactive
>
> This is slightly weird to me as "active" is not so much a state of the
> bookmark (ie a bit on the bookmark that allows you to mark some set of
> bookmarks active/inactive) but a state of a repository (bookmark x | nil
> is active).

I actually borrowed the language from hg help bookmarks, which has

  -i --inactive    mark a bookmark inactive

>   I think I might prefer:
>
>    deactivating bookmark c
>
> Or even:
>
>    leaving bookmark c

I think I like this the most, with parentheses.

$ hg update -C .
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
(leaving bookmark c)

>
> ..which gives more of a sense of 'why'. We might even want to add
> parentheses as well to emphasize that all is normal.
>
Matt Mackall - May 15, 2014, 5:42 p.m.
On Thu, 2014-05-15 at 09:44 -0700, Siddharth Agarwal wrote:
> 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> (leaving bookmark c)

Ok, looks good.
Pierre-Yves David - May 16, 2014, 5:46 p.m.
On 05/15/2014 05:50 AM, Stephen Lee wrote:
>
> On 15 May 2014 16:22, "Pierre-Yves David"
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>  >
>  >
>  >
>  > On 05/14/2014 12:50 PM, Siddharth Agarwal wrote:
>  >>
>  >> # HG changeset patch
>  >> # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>  >> # Date 1400096995 25200
>  >> #      Wed May 14 12:49:55 2014 -0700
>  >> # Node ID 8ec9ebbd09addc74ae17cc5f2ea13cb4c732a286
>  >> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
>  >> update: when deactivating a bookmark, print a message
>  >>
>  >> This helps prevent user confusion when innocent-seeming commands like
>  >> 'hg update -C .' are run.
>  >
>  >
>  > I love the idea. I always got bitten by bookmark activation or
> desactivation as a side effect of command. This should help a bit on
> this topic.
>  >
>  > I would even encourage to add more of this when we automatically
> enable it too.
>
> I sent a patch to do this on March 11.
> If it won't apply anymore  I can resend tomorrow.

If it is two months old, you better resend it.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5901,6 +5901,9 @@ 
     elif brev in repo._bookmarks:
         bookmarks.setcurrent(repo, brev)
     elif brev:
+        if repo._bookmarkcurrent:
+            ui.status(_("marking bookmark %s inactive\n") %
+                      repo._bookmarkcurrent)
         bookmarks.unsetcurrent(repo)
 
     return ret
diff --git a/tests/test-bookmarks-merge.t b/tests/test-bookmarks-merge.t
--- a/tests/test-bookmarks-merge.t
+++ b/tests/test-bookmarks-merge.t
@@ -32,6 +32,7 @@ 
 
   $ hg up -C 3
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  marking bookmark c inactive
   $ echo d > d
   $ hg add d
   $ hg commit -m'd'
@@ -54,6 +55,7 @@ 
 
   $ hg up -C 4
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark e inactive
   $ hg merge
   abort: heads are bookmarked - please merge with an explicit rev
   (run 'hg heads' to see all heads)
@@ -72,6 +74,7 @@ 
 
   $ hg up -C 4
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark e inactive
   $ echo f > f
   $ hg commit -Am "f"
   adding f
@@ -114,6 +117,7 @@ 
 
   $ hg up -C 6
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark e inactive
   $ echo g > g
   $ hg commit -Am 'g'
   adding g
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
@@ -411,6 +411,7 @@ 
   $ hg commit -m 'add bar'
   $ hg co "tip^"
   0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark @ inactive
   $ hg book add-foo
   $ hg book -r tip add-bar
 Note: this push *must* push only a single changeset, as that's the point
diff --git a/tests/test-bookmarks-strip.t b/tests/test-bookmarks-strip.t
--- a/tests/test-bookmarks-strip.t
+++ b/tests/test-bookmarks-strip.t
@@ -38,6 +38,7 @@ 
 
   $ hg update -r -2
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  marking bookmark test2 inactive
 
   $ echo eee>>qqq.txt
 
diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
--- a/tests/test-bookmarks.t
+++ b/tests/test-bookmarks.t
@@ -582,6 +582,7 @@ 
   $ hg book should-end-on-two
   $ hg co --clean 4
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark should-end-on-two inactive
   $ hg book four
   $ hg --config extensions.mq= strip 3
   saved backup bundle to * (glob)
diff --git a/tests/test-commandserver.py.out b/tests/test-commandserver.py.out
--- a/tests/test-commandserver.py.out
+++ b/tests/test-commandserver.py.out
@@ -177,6 +177,7 @@ 
 
  runcommand update -C 0
 1 files updated, 0 files merged, 2 files removed, 0 files unresolved
+marking bookmark bm3 inactive
  runcommand commit -Am. a
 created new head
  runcommand log -Gq
diff --git a/tests/test-convert-hg-source.t b/tests/test-convert-hg-source.t
--- a/tests/test-convert-hg-source.t
+++ b/tests/test-convert-hg-source.t
@@ -24,6 +24,7 @@ 
   $ hg ci -m 'merge local copy' -d '3 0'
   $ hg up -C 1
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark premerge1 inactive
   $ hg bookmark premerge2
   $ hg merge 2
   merging foo and baz to baz
diff --git a/tests/test-issue1877.t b/tests/test-issue1877.t
--- a/tests/test-issue1877.t
+++ b/tests/test-issue1877.t
@@ -34,6 +34,7 @@ 
   
   $ hg up 1e6c11564562
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark main inactive
   $ hg merge main
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
diff --git a/tests/test-rebase-bookmarks.t b/tests/test-rebase-bookmarks.t
--- a/tests/test-rebase-bookmarks.t
+++ b/tests/test-rebase-bookmarks.t
@@ -154,6 +154,7 @@ 
 
   $ hg up 2
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  marking bookmark X inactive
   $ echo 'C' > c
   $ hg add c
   $ hg ci -m 'other C'
diff --git a/tests/test-rollback.t b/tests/test-rollback.t
--- a/tests/test-rollback.t
+++ b/tests/test-rollback.t
@@ -82,6 +82,7 @@ 
   0  default  add a again
   $ hg update default
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  marking bookmark foo inactive
   $ hg bookmark bar
   $ cat .hg/undo.branch ; echo
   test
diff --git a/tests/test-subrepo-svn.t b/tests/test-subrepo-svn.t
--- a/tests/test-subrepo-svn.t
+++ b/tests/test-subrepo-svn.t
@@ -470,6 +470,7 @@ 
   $ hg book other
   $ hg co -r 'p1(tip)'
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  marking bookmark other inactive
   $ echo "obstruct =        [svn]       $SVNREPOURL/src" >> .hgsub
   $ svn co -r5 --quiet "$SVNREPOURL"/src obstruct
   $ hg commit -m 'Other branch which will be obstructed'
@@ -543,6 +544,7 @@ 
   A    *recreated/somethingold (glob)
   Checked out revision 10.
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  marking bookmark other inactive
   $ test -f recreated/somethingold
 
 Test archive