Patchwork [V2] bookmarks: add 'hg pull -B .' for pulling the active bookmark (issue5258)

login
register
mail settings
Submitter liscju
Date June 20, 2016, 7:51 a.m.
Message ID <9c9bacc0c8f7c830fe05.1466409061@liscju-VirtualBox>
Download mbox | patch
Permalink /patch/15544/
State Accepted
Headers show

Comments

liscju - June 20, 2016, 7:51 a.m.
# HG changeset patch
# User liscju <piotr.listkiewicz@gmail.com>
# Date 1464814737 -7200
#      Wed Jun 01 22:58:57 2016 +0200
# Node ID 9c9bacc0c8f7c830fe05ff47442d85ac9576fa04
# Parent  d269e7db2f55f812c9ee1efe5626602614174df3
bookmarks: add 'hg pull -B .' for pulling the active bookmark (issue5258)
Yuya Nishihara - June 20, 2016, 1:09 p.m.
On Mon, 20 Jun 2016 09:51:01 +0200, liscju wrote:
> # HG changeset patch
> # User liscju <piotr.listkiewicz@gmail.com>
> # Date 1464814737 -7200
> #      Wed Jun 01 22:58:57 2016 +0200
> # Node ID 9c9bacc0c8f7c830fe05ff47442d85ac9576fa04
> # Parent  d269e7db2f55f812c9ee1efe5626602614174df3
> bookmarks: add 'hg pull -B .' for pulling the active bookmark (issue5258)

Queued this, thanks. Can you update the help message as a follow-up?

> +++ b/mercurial/exchange.py
> @@ -1056,7 +1056,8 @@ class pulloperation(object):
>          # revision we try to pull (None is "all")
>          self.heads = heads
>          # bookmark pulled explicitly
> -        self.explicitbookmarks = bookmarks
> +        self.explicitbookmarks = [repo._bookmarks.expandname(bookmark)
> +                                  for bookmark in bookmarks]

I have no idea where '.' should be expanded, but it seems inconsistent that
pulloperation keeps expanded names but pushoperation doesn't. Thoughts?
Sean Farley - June 20, 2016, 9:15 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> On Mon, 20 Jun 2016 09:51:01 +0200, liscju wrote:
>> # HG changeset patch
>> # User liscju <piotr.listkiewicz@gmail.com>
>> # Date 1464814737 -7200
>> #      Wed Jun 01 22:58:57 2016 +0200
>> # Node ID 9c9bacc0c8f7c830fe05ff47442d85ac9576fa04
>> # Parent  d269e7db2f55f812c9ee1efe5626602614174df3
>> bookmarks: add 'hg pull -B .' for pulling the active bookmark (issue5258)
>
> Queued this, thanks. Can you update the help message as a follow-up?
>
>> +++ b/mercurial/exchange.py
>> @@ -1056,7 +1056,8 @@ class pulloperation(object):
>>          # revision we try to pull (None is "all")
>>          self.heads = heads
>>          # bookmark pulled explicitly
>> -        self.explicitbookmarks = bookmarks
>> +        self.explicitbookmarks = [repo._bookmarks.expandname(bookmark)
>> +                                  for bookmark in bookmarks]
>
> I have no idea where '.' should be expanded, but it seems inconsistent that
> pulloperation keeps expanded names but pushoperation doesn't. Thoughts?

To add from a previous mail about this, I think we should expand '.' in
the bookmark class (or util) and permanently reserve '.' from all
namespaces. Are there any corner-cases to think about here?
Pierre-Yves David - June 21, 2016, 12:51 a.m.
On 06/20/2016 11:15 PM, Sean Farley wrote:
> Yuya Nishihara <yuya@tcha.org> writes:
> 
>> On Mon, 20 Jun 2016 09:51:01 +0200, liscju wrote:
>>> # HG changeset patch
>>> # User liscju <piotr.listkiewicz@gmail.com>
>>> # Date 1464814737 -7200
>>> #      Wed Jun 01 22:58:57 2016 +0200
>>> # Node ID 9c9bacc0c8f7c830fe05ff47442d85ac9576fa04
>>> # Parent  d269e7db2f55f812c9ee1efe5626602614174df3
>>> bookmarks: add 'hg pull -B .' for pulling the active bookmark (issue5258)
>>
>> Queued this, thanks. Can you update the help message as a follow-up?
>>
>>> +++ b/mercurial/exchange.py
>>> @@ -1056,7 +1056,8 @@ class pulloperation(object):
>>>          # revision we try to pull (None is "all")
>>>          self.heads = heads
>>>          # bookmark pulled explicitly
>>> -        self.explicitbookmarks = bookmarks
>>> +        self.explicitbookmarks = [repo._bookmarks.expandname(bookmark)
>>> +                                  for bookmark in bookmarks]
>>
>> I have no idea where '.' should be expanded, but it seems inconsistent that
>> pulloperation keeps expanded names but pushoperation doesn't. Thoughts?
> 
> To add from a previous mail about this, I think we should expand '.' in
> the bookmark class (or util) and permanently reserve '.' from all
> namespaces. Are there any corner-cases to think about here?

That would seems very wise. Let's do that.
Sean Farley - June 21, 2016, 1:51 a.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> On 06/20/2016 11:15 PM, Sean Farley wrote:
>> Yuya Nishihara <yuya@tcha.org> writes:
>> 
>>> On Mon, 20 Jun 2016 09:51:01 +0200, liscju wrote:
>>>> # HG changeset patch
>>>> # User liscju <piotr.listkiewicz@gmail.com>
>>>> # Date 1464814737 -7200
>>>> #      Wed Jun 01 22:58:57 2016 +0200
>>>> # Node ID 9c9bacc0c8f7c830fe05ff47442d85ac9576fa04
>>>> # Parent  d269e7db2f55f812c9ee1efe5626602614174df3
>>>> bookmarks: add 'hg pull -B .' for pulling the active bookmark (issue5258)
>>>
>>> Queued this, thanks. Can you update the help message as a follow-up?
>>>
>>>> +++ b/mercurial/exchange.py
>>>> @@ -1056,7 +1056,8 @@ class pulloperation(object):
>>>>          # revision we try to pull (None is "all")
>>>>          self.heads = heads
>>>>          # bookmark pulled explicitly
>>>> -        self.explicitbookmarks = bookmarks
>>>> +        self.explicitbookmarks = [repo._bookmarks.expandname(bookmark)
>>>> +                                  for bookmark in bookmarks]
>>>
>>> I have no idea where '.' should be expanded, but it seems inconsistent that
>>> pulloperation keeps expanded names but pushoperation doesn't. Thoughts?
>> 
>> To add from a previous mail about this, I think we should expand '.' in
>> the bookmark class (or util) and permanently reserve '.' from all
>> namespaces. Are there any corner-cases to think about here?
>
> That would seems very wise. Let's do that.
>
> -- 
> Pierre-Yves DAvid

FYI, it seems your signature has a typo in your last name (unless you
really meant 'DAvid').
Yuya Nishihara - June 21, 2016, 12:34 p.m.
On Mon, 20 Jun 2016 14:15:30 -0700, Sean Farley wrote:
> >> +++ b/mercurial/exchange.py
> >> @@ -1056,7 +1056,8 @@ class pulloperation(object):
> >>          # revision we try to pull (None is "all")
> >>          self.heads = heads
> >>          # bookmark pulled explicitly
> >> -        self.explicitbookmarks = bookmarks
> >> +        self.explicitbookmarks = [repo._bookmarks.expandname(bookmark)
> >> +                                  for bookmark in bookmarks]  
> >
> > I have no idea where '.' should be expanded, but it seems inconsistent that
> > pulloperation keeps expanded names but pushoperation doesn't. Thoughts?  
> 
> To add from a previous mail about this, I think we should expand '.' in
> the bookmark class (or util) and permanently reserve '.' from all
> namespaces. Are there any corner-cases to think about here?

I feel like saying it sounds nice, but actually I don't get it. The bookmark
class isn't involved to process the explicitbookmarks in updatefromremote().
liscju - June 21, 2016, 12:48 p.m.
>
> I feel like saying it sounds nice, but actually I don't get it. The
> bookmark
> class isn't involved to process the explicitbookmarks in
> updatefromremote().


I think it could be done by supporting '.' in all functions in bookmark
module - including updatefromremote function which was used here and
bmstore not used in this case.


2016-06-21 14:34 GMT+02:00 Yuya Nishihara <yuya@tcha.org>:

> On Mon, 20 Jun 2016 14:15:30 -0700, Sean Farley wrote:
> > >> +++ b/mercurial/exchange.py
> > >> @@ -1056,7 +1056,8 @@ class pulloperation(object):
> > >>          # revision we try to pull (None is "all")
> > >>          self.heads = heads
> > >>          # bookmark pulled explicitly
> > >> -        self.explicitbookmarks = bookmarks
> > >> +        self.explicitbookmarks =
> [repo._bookmarks.expandname(bookmark)
> > >> +                                  for bookmark in bookmarks]
> > >
> > > I have no idea where '.' should be expanded, but it seems inconsistent
> that
> > > pulloperation keeps expanded names but pushoperation doesn't. Thoughts?
> >
> > To add from a previous mail about this, I think we should expand '.' in
> > the bookmark class (or util) and permanently reserve '.' from all
> > namespaces. Are there any corner-cases to think about here?
>
> I feel like saying it sounds nice, but actually I don't get it. The
> bookmark
> class isn't involved to process the explicitbookmarks in
> updatefromremote().
>
Yuya Nishihara - June 21, 2016, 2:04 p.m.
On Tue, 21 Jun 2016 14:48:09 +0200, Piotr Listkiewicz wrote:
> > I feel like saying it sounds nice, but actually I don't get it. The
> > bookmark
> > class isn't involved to process the explicitbookmarks in
> > updatefromremote().  
> 
> I think it could be done by supporting '.' in all functions in bookmark
> module - including updatefromremote function which was used here and
> bmstore not used in this case.

IMHO, if we don't have clear boundary between '.' and <real-bookmark>, we
would face a bunch of bugs related to the '.' bookmark.
liscju - June 21, 2016, 3:15 p.m.
>
> IMHO, if we don't have clear boundary between '.' and <real-bookmark>, we
> would face a bunch of bugs related to the '.' bookmark.


Totally agree but i have no idea how to make a clear boundary between them.
Expanding name in all functions in bookmark module would definetely be
error prone.


2016-06-21 16:04 GMT+02:00 Yuya Nishihara <yuya@tcha.org>:

> On Tue, 21 Jun 2016 14:48:09 +0200, Piotr Listkiewicz wrote:
> > > I feel like saying it sounds nice, but actually I don't get it. The
> > > bookmark
> > > class isn't involved to process the explicitbookmarks in
> > > updatefromremote().
> >
> > I think it could be done by supporting '.' in all functions in bookmark
> > module - including updatefromremote function which was used here and
> > bmstore not used in this case.
>
> IMHO, if we don't have clear boundary between '.' and <real-bookmark>, we
> would face a bunch of bugs related to the '.' bookmark.
>

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5805,6 +5805,7 @@  def pull(ui, repo, source="default", **o
             remotebookmarks = other.listkeys('bookmarks')
             pullopargs['remotebookmarks'] = remotebookmarks
             for b in opts['bookmark']:
+                b = repo._bookmarks.expandname(b)
                 if b not in remotebookmarks:
                     raise error.Abort(_('remote bookmark %s not found!') % b)
                 revs.append(remotebookmarks[b])
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1056,7 +1056,8 @@  class pulloperation(object):
         # revision we try to pull (None is "all")
         self.heads = heads
         # bookmark pulled explicitly
-        self.explicitbookmarks = bookmarks
+        self.explicitbookmarks = [repo._bookmarks.expandname(bookmark)
+                                  for bookmark in bookmarks]
         # do we force pull?
         self.force = force
         # whether a streaming clone was requested
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
@@ -252,7 +252,10 @@  divergent bookmarks
 
 explicit pull should overwrite the local version (issue4439)
 
-  $ hg pull --config paths.foo=../a foo -B X
+  $ hg update -r X
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  (activating bookmark X)
+  $ hg pull --config paths.foo=../a foo -B .
   pulling from $TESTTMP/a (glob)
   no changes found
   divergent bookmark @ stored as @foo
@@ -366,7 +369,10 @@  Update a bookmark right after the initia
      X                         1:0d2164f0ce0d
    * Y                         5:35d1ef0a8d1b
      Z                         1:0d2164f0ce0d
-  $ hg pull -B Y
+  $ hg update -r Y
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  (activating bookmark Y)
+  $ hg pull -B .
   pulling from http://localhost:$HGPORT/
   searching for changes
   adding changesets
@@ -376,9 +382,9 @@  Update a bookmark right after the initia
   updating bookmark Y
   (run 'hg update' to get a working copy)
   $ hg book
-   * @                         1:0d2164f0ce0d
+     @                         1:0d2164f0ce0d
      X                         1:0d2164f0ce0d
-     Y                         5:35d1ef0a8d1b
+   * Y                         5:35d1ef0a8d1b
      Z                         1:0d2164f0ce0d
 
 (done with this section of the test)