D7384: commands: necessary annotations and suppresssions to pass pytype

[dlax (Denis Laxalde)]

dlax added a comment.


  In D7384#118739 , @marmoute 
wrote:
  
  > What's up on this ? It seemed on a good track, but I don't think it landed. 
@dlax I think you offer to use a context manager got a warm welcome, I would 
says, go ahead with it.
  
  The context manager got in through D7430 
.

INLINE COMMENTS

> commands.py:4741
> +# pytype isn't convinced, so we have to suppress the
> +# warning about list not having a max() method.
>  endrev = revs.max() + 1

I think this comment should be removed since the `# pytype: disable` got 
removed in the last version of the diff.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: marmoute, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7980: resourceutil: ensure `_rootpath` is defined under py2exe

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> resourceutil.py:39
>  datapath = os.path.dirname(os.path.dirname(pycompat.fsencode(__file__)))
>  _rootpath = os.path.dirname(datapath)
>  

`_rootpath` declaration could be moved after the `else` clause to avoid 
repetition.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7980/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7980

To: mharbison72, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7682: patch: make __repr__() return str

[dlax (Denis Laxalde)]

dlax added inline comments.
dlax accepted this revision.

INLINE COMMENTS

> patch.py:966
>  def __repr__(self):
> -return b'' % (b' '.join(map(repr, self.files(
> +return '' % (' '.join(map(repr, self.files(
>  

Somehow unrelated, but `self.files()` returns a list of bytes I think. So this 
will lead to something like `""`. But maybe this is 
okay? (at least it wouldn't crash now)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7682/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7682

To: mharbison72, #hg-reviewers, pulkit, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7633: clone: extract helper for checking mutually exclusive args

[dlax (Denis Laxalde)]

dlax added a comment.


  Useful refactoring!

INLINE COMMENTS

> cmdutil.py:263
>  
> +def check_unique_argument(opts, *args):
> +"""abort if more than one of the arguments are in opts"""

The function name is a bit misleading; it looks like it would check that an 
option wasn't specified multiple times. Perhaps `check_exclusive_arguments`?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7633/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7633

To: martinvonz, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7620: merge: add commands.merge.require-rev to require an argument to hg merge

[dlax (Denis Laxalde)]

This revision now requires changes to proceed.
dlax added a comment.
dlax requested changes to this revision.


  This should be documented in `mercurial/helptext/config.txt` I think.
  
  Otherwise, this look sensible to me.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7620/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7620

To: spectral, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax added a comment.


  In D7506#111947 , @spectral 
wrote:
  
  > I don't think `from . import show` works generally.
  
  I did that because `test-check-module-imports.t` complained otherwise when 
using `from hgext import show`:
  
hgext/phabricator.py:89: import should be relative: hgext
  
  Also, there's already a similar `from . import rebase` in `hgext/split.py` so 
I thought that relative import was fine.
  
  I'm happy to change if there's a solution, though.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

To: dlax, #hg-reviewers, pulkit
Cc: spectral, mharbison72, mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

Closed by commit rHG70060915c3f2: phabricator: add a phabstatus 
show view (authored by dlax).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7506?vs=18589=18603

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -11,6 +11,10 @@
 revisions in a format suitable for :hg:`import`, and a ``phabupdate`` command
 to update statuses in batch.
 
+A "phabstatus" view for :hg:`show` is also provided; it displays status
+information of Phabricator differentials associated with unfinished
+changesets.
+
 By default, Phabricator requires ``Test Plan`` which might prevent some
 changeset from being sent. The requirement could be disabled by changing
 ``differential.require-test-plan-field`` config server side.
@@ -60,7 +64,9 @@
 encoding,
 error,
 exthelper,
+graphmod,
 httpconnection as httpconnectionmod,
+logcmdutil,
 match,
 mdiff,
 obsutil,
@@ -80,6 +86,8 @@
 procutil,
 stringutil,
 )
+from . import show
+
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' 
for
 # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
@@ -465,6 +473,29 @@
 return result
 
 
+def getdrevmap(repo, revs):
+"""Return a dict mapping each rev in `revs` to their Differential Revision
+ID or None.
+"""
+result = {}
+for rev in revs:
+result[rev] = None
+ctx = repo[rev]
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+result[rev] = int(m.group('id'))
+continue
+# Check tags
+for tag in repo.nodetags(ctx.node()):
+m = _differentialrevisiontagre.match(tag)
+if m:
+result[rev] = int(m.group(1))
+break
+
+return result
+
+
 def getdiff(ctx, diffopts):
 """plain-text diff without header (user, commit message, etc)"""
 output = util.stringio()
@@ -1651,3 +1682,42 @@
 
 return templateutil.hybriddict({b'url': url, b'id': t,})
 return None
+
+
+@show.showview(b'phabstatus', csettopic=b'work')
+def phabstatusshowview(ui, repo, displayer):
+"""Phabricator differiential status"""
+revs = repo.revs('sort(_underway(), topo)')
+drevmap = getdrevmap(repo, revs)
+unknownrevs, drevids, revsbydrevid = [], set([]), {}
+for rev, drevid in pycompat.iteritems(drevmap):
+if drevid is not None:
+drevids.add(drevid)
+revsbydrevid.setdefault(drevid, set([])).add(rev)
+else:
+unknownrevs.append(rev)
+
+drevs = callconduit(ui, b'differential.query', {b'ids': list(drevids)})
+drevsbyrev = {}
+for drev in drevs:
+for rev in revsbydrevid[int(drev[b'id'])]:
+drevsbyrev[rev] = drev
+
+def phabstatus(ctx):
+drev = drevsbyrev[ctx.rev()]
+ui.write(b"\n%(uri)s %(statusName)s\n" % drev)
+
+revs -= smartset.baseset(unknownrevs)
+revdag = graphmod.dagwalker(repo, revs)
+
+ui.setconfig(b'experimental', b'graphshorten', True)
+displayer._exthook = phabstatus
+nodelen = show.longestshortest(repo, revs)
+logcmdutil.displaygraph(
+ui,
+repo,
+revdag,
+displayer,
+graphmod.asciiedges,
+props={b'nodelen': nodelen},
+)



To: dlax, #hg-reviewers, pulkit
Cc: mharbison72, mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7507: phabricator: add a "phabstatus" template keyword

[dlax (Denis Laxalde)]

Closed by commit rHG79c0121220e3: phabricator: add a phabstatus 
template keyword (authored by dlax).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7507?vs=18590=18604

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7507/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7507

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1684,6 +1684,28 @@
 return None
 
 
+@eh.templatekeyword(b'phabstatus', requires={b'ctx', b'repo', b'ui'})
+def template_status(context, mapping):
+""":phabstatus: String. Status of Phabricator differential.
+"""
+ctx = context.resource(mapping, b'ctx')
+repo = context.resource(mapping, b'repo')
+ui = context.resource(mapping, b'ui')
+
+rev = ctx.rev()
+try:
+drevid = getdrevmap(repo, [rev])[rev]
+except KeyError:
+return None
+drevs = callconduit(ui, b'differential.query', {b'ids': [drevid]})
+for drev in drevs:
+if int(drev[b'id']) == drevid:
+return templateutil.hybriddict(
+{b'url': drev[b'uri'], b'status': drev[b'statusName'],}
+)
+return None
+
+
 @show.showview(b'phabstatus', csettopic=b'work')
 def phabstatusshowview(ui, repo, displayer):
 """Phabricator differiential status"""



To: dlax, #hg-reviewers, pulkit
Cc: mharbison72, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7507: phabricator: add a "phabstatus" template keyword

[dlax (Denis Laxalde)]

dlax updated this revision to Diff 18590.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7507?vs=18321=18590

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7507/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7507

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1684,6 +1684,28 @@
 return None
 
 
+@eh.templatekeyword(b'phabstatus', requires={b'ctx', b'repo', b'ui'})
+def template_status(context, mapping):
+""":phabstatus: String. Status of Phabricator differential.
+"""
+ctx = context.resource(mapping, b'ctx')
+repo = context.resource(mapping, b'repo')
+ui = context.resource(mapping, b'ui')
+
+rev = ctx.rev()
+try:
+drevid = getdrevmap(repo, [rev])[rev]
+except KeyError:
+return None
+drevs = callconduit(ui, b'differential.query', {b'ids': [drevid]})
+for drev in drevs:
+if int(drev[b'id']) == drevid:
+return templateutil.hybriddict(
+{b'url': drev[b'uri'], b'status': drev[b'statusName'],}
+)
+return None
+
+
 @show.showview(b'phabstatus', csettopic=b'work')
 def phabstatusshowview(ui, repo, displayer):
 """Phabricator differiential status"""



To: dlax, #hg-reviewers, pulkit
Cc: mharbison72, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax updated this revision to Diff 18589.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7506?vs=18338=18589

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -11,6 +11,10 @@
 revisions in a format suitable for :hg:`import`, and a ``phabupdate`` command
 to update statuses in batch.
 
+A "phabstatus" view for :hg:`show` is also provided; it displays status
+information of Phabricator differentials associated with unfinished
+changesets.
+
 By default, Phabricator requires ``Test Plan`` which might prevent some
 changeset from being sent. The requirement could be disabled by changing
 ``differential.require-test-plan-field`` config server side.
@@ -60,7 +64,9 @@
 encoding,
 error,
 exthelper,
+graphmod,
 httpconnection as httpconnectionmod,
+logcmdutil,
 match,
 mdiff,
 obsutil,
@@ -80,6 +86,8 @@
 procutil,
 stringutil,
 )
+from . import show
+
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' 
for
 # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
@@ -465,6 +473,29 @@
 return result
 
 
+def getdrevmap(repo, revs):
+"""Return a dict mapping each rev in `revs` to their Differential Revision
+ID or None.
+"""
+result = {}
+for rev in revs:
+result[rev] = None
+ctx = repo[rev]
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+result[rev] = int(m.group('id'))
+continue
+# Check tags
+for tag in repo.nodetags(ctx.node()):
+m = _differentialrevisiontagre.match(tag)
+if m:
+result[rev] = int(m.group(1))
+break
+
+return result
+
+
 def getdiff(ctx, diffopts):
 """plain-text diff without header (user, commit message, etc)"""
 output = util.stringio()
@@ -1651,3 +1682,42 @@
 
 return templateutil.hybriddict({b'url': url, b'id': t,})
 return None
+
+
+@show.showview(b'phabstatus', csettopic=b'work')
+def phabstatusshowview(ui, repo, displayer):
+"""Phabricator differiential status"""
+revs = repo.revs('sort(_underway(), topo)')
+drevmap = getdrevmap(repo, revs)
+unknownrevs, drevids, revsbydrevid = [], set([]), {}
+for rev, drevid in pycompat.iteritems(drevmap):
+if drevid is not None:
+drevids.add(drevid)
+revsbydrevid.setdefault(drevid, set([])).add(rev)
+else:
+unknownrevs.append(rev)
+
+drevs = callconduit(ui, b'differential.query', {b'ids': list(drevids)})
+drevsbyrev = {}
+for drev in drevs:
+for rev in revsbydrevid[int(drev[b'id'])]:
+drevsbyrev[rev] = drev
+
+def phabstatus(ctx):
+drev = drevsbyrev[ctx.rev()]
+ui.write(b"\n%(uri)s %(statusName)s\n" % drev)
+
+revs -= smartset.baseset(unknownrevs)
+revdag = graphmod.dagwalker(repo, revs)
+
+ui.setconfig(b'experimental', b'graphshorten', True)
+displayer._exthook = phabstatus
+nodelen = show.longestshortest(repo, revs)
+logcmdutil.displaygraph(
+ui,
+repo,
+revdag,
+displayer,
+graphmod.asciiedges,
+props={b'nodelen': nodelen},
+)



To: dlax, #hg-reviewers, pulkit
Cc: mharbison72, mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7505: logcmdutil: call _exthook() in changesettemplater

[dlax (Denis Laxalde)]

Closed by commit rHG6331a6fc3304: logcmdutil: call _exthook() in 
changesettemplater (authored by dlax).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7505?vs=18312=18581

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7505/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7505

AFFECTED FILES
  mercurial/logcmdutil.py

CHANGE DETAILS

diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -598,6 +598,7 @@
 # write changeset metadata, then patch if requested
 key = self._parts[self._tref]
 self.ui.write(self.t.render(key, props))
+self._exthook(ctx)
 self._showpatch(ctx, graphwidth)
 
 if self._parts[b'footer']:



To: dlax, #hg-reviewers, pulkit
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7513: phabricator: fix processing of tags/desc in getoldnodedrevmap()

[dlax (Denis Laxalde)]

Closed by commit rHG16b607e9f714: phabricator: fix processing of tags/desc in 
getoldnodedrevmap() (authored by dlax).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7513?vs=18339=18580

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7513/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7513

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -403,12 +403,15 @@
 m = _differentialrevisiontagre.match(tag)
 if m:
 toconfirm[node] = (0, set(precnodes), int(m.group(1)))
-continue
-
-# Check commit message
-m = _differentialrevisiondescre.search(ctx.description())
-if m:
-toconfirm[node] = (1, set(precnodes), int(m.group('id')))
+break
+else:
+continue  # move to next predecessor
+break  # found a tag, stop
+else:
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+toconfirm[node] = (1, set(precnodes), int(m.group('id')))
 
 # Double check if tags are genuine by collecting all old nodes from
 # Phabricator, and expect precursors overlap with it.



To: dlax, #hg-reviewers, pulkit
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7540: tests: cover revision conversion logic in githelp tests

[dlax (Denis Laxalde)]

Closed by commit rHG5470e63686ca: tests: cover revision conversion logic in 
githelp tests (authored by dlax).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7540?vs=18417=18432

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7540/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7540

AFFECTED FILES
  tests/test-githelp.t

CHANGE DETAILS

diff --git a/tests/test-githelp.t b/tests/test-githelp.t
--- a/tests/test-githelp.t
+++ b/tests/test-githelp.t
@@ -264,6 +264,10 @@
   $ hg githelp -- git commit --reuse-message deadbeef
   hg commit -M deadbeef
 
+githelp for reuse message using HEAD
+  $ hg githelp -- git commit --reuse-message HEAD~
+  hg commit -M .~1
+
 githelp for apply with no options
   $ hg githelp -- apply
   hg import --no-commit



To: dlax, #hg-reviewers, pulkit, mharbison72
Cc: mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7540: tests: cover revision conversion logic in githelp tests

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  There was no test involving actual conversion of option values when they
  contain a git revision name (to be converted as a hg one by
  hgext.githelp.convert()). Adding one. This test would fail on Python 3
  without https://phab.mercurial-scm.org/D7537.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7540

AFFECTED FILES
  tests/test-githelp.t

CHANGE DETAILS

diff --git a/tests/test-githelp.t b/tests/test-githelp.t
--- a/tests/test-githelp.t
+++ b/tests/test-githelp.t
@@ -264,6 +264,10 @@
   $ hg githelp -- git commit --reuse-message deadbeef
   hg commit -M deadbeef
 
+githelp for reuse message using HEAD
+  $ hg githelp -- git commit --reuse-message HEAD~
+  hg commit -M .~1
+
 githelp for apply with no options
   $ hg githelp -- apply
   hg import --no-commit



To: dlax, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7537: githelp: fix a `str` type conditional for py3

[dlax (Denis Laxalde)]

dlax added a comment.
dlax accepted this revision.


  There is apparently no test coverage for this.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7537/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7537

To: mharbison72, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7533: repair: fix an `isinstance(nodelist, str)` check for py3

[dlax (Denis Laxalde)]

dlax added a comment.
dlax accepted this revision.


  All these API where one can pass either a list of "things" or just one 
"thing" is kind of ugly. We should only handle the list case, I think.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7533/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7533

To: mharbison72, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7517: filemerge: byteify the open() mode

[dlax (Denis Laxalde)]

dlax added a comment.


  > This is actually pycompat.open(), so it need bytes.
  
  I don't understand why this is needed. The default value for "mode" as bytes 
comes from a407f9009392 
, 
but I don't understand the rationale.
  Shouldn't we instead change all calls to `pycompat.open()` to use a native 
str for `mode`? (and drop `sysstr(mode)` in `pycompat.open()`).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7517/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7517

To: mharbison72, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7516: webutil: add missing argument to join()

[dlax (Denis Laxalde)]

dlax added a comment.
dlax accepted this revision.


  Looks like dead code

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7516/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7516

To: mharbison72, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7507: phabricator: add a "phabstatus" template keyword

[dlax (Denis Laxalde)]

dlax added a comment.


  >   I only have a superficial understanding about how templates work, but I 
assume that there's no global pre-resolution step where a single query could be 
done and the results stuffed into the context or something, is there?
  
  Not that I am aware. Using this keyword on large revsets is indeed slow.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7507/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7507

To: dlax, #hg-reviewers
Cc: mharbison72, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7513: phabricator: fix processing of tags/desc in getoldnodedrevmap()

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  It seems that the previous logic was wrong (it essentially comes
  from changeset 3ab0d5767b54 
 
where the result got accumulated instead of
  early returned).
  First of all, the "continue" in first "if m:" is useless because we're
  at the end of the loop. Then, the algorithm seems weird because we will
  process all predecessors of a node and possibly override
  `toconfirm[node]` for each of these having a tag (maybe this doesn't
  happen, but still). Finally, we would also override `toconfirm[node]`
  when the "Differential Revision: " is found in changeset description.
  Maybe this is not a big deal when there is no mix of local tag and
  changeset description update?
  
  The logic is changed so that the loop on predecessors stops upon first
  match of a tag and so that the changeset description is only checked if
  no tag was found. Therefore, `toconfirm[node]` is only set once.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7513

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -403,12 +403,15 @@
 m = _differentialrevisiontagre.match(tag)
 if m:
 toconfirm[node] = (0, set(precnodes), int(m.group(1)))
-continue
-
-# Check commit message
-m = _differentialrevisiondescre.search(ctx.description())
-if m:
-toconfirm[node] = (1, set(precnodes), int(m.group('id')))
+break
+else:
+continue  # move to next predecessor
+break  # found a tag, stop
+else:
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+toconfirm[node] = (1, set(precnodes), int(m.group('id')))
 
 # Double check if tags are genuine by collecting all old nodes from
 # Phabricator, and expect precursors overlap with it.



To: dlax, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax edited the summary of this revision.
dlax updated this revision to Diff 18338.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7506?vs=18320=18338

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -11,6 +11,10 @@
 revisions in a format suitable for :hg:`import`, and a ``phabupdate`` command
 to update statuses in batch.
 
+A "phabstatus" view for :hg:`show` is also provided; it displays status
+information of Phabricator differentials associated with unfinished
+changesets.
+
 By default, Phabricator requires ``Test Plan`` which might prevent some
 changeset from being sent. The requirement could be disabled by changing
 ``differential.require-test-plan-field`` config server side.
@@ -60,9 +64,11 @@
 encoding,
 error,
 exthelper,
+graphmod,
 httpconnection as httpconnectionmod,
 match,
 mdiff,
+logcmdutil,
 obsutil,
 parser,
 patch,
@@ -80,6 +86,11 @@
 procutil,
 stringutil,
 )
+from hgext.show import (
+longestshortest,
+showview,
+)
+
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' 
for
 # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
@@ -462,6 +473,29 @@
 return result
 
 
+def getdrevmap(repo, revs):
+"""Return a dict mapping each rev in `revs` to their Differential Revision
+ID or None.
+"""
+result = {}
+for rev in revs:
+result[rev] = None
+ctx = repo[rev]
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+result[rev] = int(m.group('id'))
+continue
+# Check tags
+for tag in repo.nodetags(ctx.node()):
+m = _differentialrevisiontagre.match(tag)
+if m:
+result[rev] = int(m.group(1))
+break
+
+return result
+
+
 def getdiff(ctx, diffopts):
 """plain-text diff without header (user, commit message, etc)"""
 output = util.stringio()
@@ -1648,3 +1682,42 @@
 
 return templateutil.hybriddict({b'url': url, b'id': t,})
 return None
+
+
+@showview(b'phabstatus', csettopic=b'work')
+def phabstatusshowview(ui, repo, displayer):
+"""Phabricator differiential status"""
+revs = repo.revs('sort(_underway(), topo)')
+drevmap = getdrevmap(repo, revs)
+unknownrevs, drevids, revsbydrevid = [], set([]), {}
+for rev, drevid in pycompat.iteritems(drevmap):
+if drevid is not None:
+drevids.add(drevid)
+revsbydrevid.setdefault(drevid, set([])).add(rev)
+else:
+unknownrevs.append(rev)
+
+drevs = callconduit(ui, b'differential.query', {b'ids': list(drevids)})
+drevsbyrev = {}
+for drev in drevs:
+for rev in revsbydrevid[int(drev[b'id'])]:
+drevsbyrev[rev] = drev
+
+def phabstatus(ctx):
+drev = drevsbyrev[ctx.rev()]
+ui.write(b"\n%(uri)s %(statusName)s\n" % drev)
+
+revs -= smartset.baseset(unknownrevs)
+revdag = graphmod.dagwalker(repo, revs)
+
+ui.setconfig(b'experimental', b'graphshorten', True)
+displayer._exthook = phabstatus
+nodelen = longestshortest(repo, revs)
+logcmdutil.displaygraph(
+ui,
+repo,
+revdag,
+displayer,
+graphmod.asciiedges,
+props={b'nodelen': nodelen},
+)



To: dlax, #hg-reviewers
Cc: mharbison72, mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax added a comment.
dlax planned changes to this revision.


  In D7506#110321 , @mharbison72 
wrote:
  
  > In D7506#110288 , @mharbison72 
wrote:
  >
  >> I'm not sure why, but this version seems to also show obsolete revisions.  
I've got a bunch of `x` and `*` nodes in hg-committed right now.  I didn't see 
that before, although that was on a different clone that I don't have access to 
now.
  >
  > After further review, the obsolete revisions have unstable children going 
way back, so that's why they show.  But I'm super confused by the output.  This 
isn't the whole output, but the middle or so that corresponds to the latest(!) 
commits:
  
  Hm, I don't have this issue. Even with visible obsolete revisions with a phab 
differential, the output seem consistent with "show work".
  The revisions shown by the "phabstatus" view is just a subset of that of 
"work" view (subset of revisions with a differential). Do you see more 
revisions in "phabstatus" than in "work" or am I misunderstanding something?
  Or perhaps this is because of a wrong sorting of the filtered set? I'll make 
sure the final is still topo sorted and send another version in a moment.
  
  > Is the goal to limit to the current stack, or all non public commits?  I 
can see a use case for both.
  
  I don't use the "stack" view often, so my goal is to map to the "work" view 
which I use more. But perhaps it'd make sense to have "hg show phabwork" and 
"hg show phabstack"? (We can rename the view, and add the other one later on.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

To: dlax, #hg-reviewers
Cc: mharbison72, mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7507: phabricator: add a "phabstatus" template keyword

[dlax (Denis Laxalde)]

dlax updated this revision to Diff 18321.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7507?vs=18316=18321

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7507/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7507

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1684,6 +1684,28 @@
 return None
 
 
+@eh.templatekeyword(b'phabstatus', requires={b'ctx', b'repo', b'ui'})
+def template_status(context, mapping):
+""":phabstatus: String. Status of Phabricator differential.
+"""
+ctx = context.resource(mapping, b'ctx')
+repo = context.resource(mapping, b'repo')
+ui = context.resource(mapping, b'ui')
+
+rev = ctx.rev()
+try:
+drevid = getdrevmap(repo, [rev])[rev]
+except KeyError:
+return None
+drevs = callconduit(ui, b'differential.query', {b'ids': [drevid]})
+for drev in drevs:
+if int(drev[b'id']) == drevid:
+return templateutil.hybriddict(
+{b'url': drev[b'uri'], b'status': drev[b'statusName'],}
+)
+return None
+
+
 @showview(b'phabstatus', csettopic=b'work')
 def phabstatusshowview(ui, repo, displayer):
 """Phabricator differiential status"""



To: dlax, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax updated this revision to Diff 18320.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7506?vs=18315=18320

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -11,6 +11,10 @@
 revisions in a format suitable for :hg:`import`, and a ``phabupdate`` command
 to update statuses in batch.
 
+A "phabstatus" view for :hg:`show` is also provided; it displays status
+information of Phabricator differentials associated with unfinished
+changesets.
+
 By default, Phabricator requires ``Test Plan`` which might prevent some
 changeset from being sent. The requirement could be disabled by changing
 ``differential.require-test-plan-field`` config server side.
@@ -60,9 +64,11 @@
 encoding,
 error,
 exthelper,
+graphmod,
 httpconnection as httpconnectionmod,
 match,
 mdiff,
+logcmdutil,
 obsutil,
 parser,
 patch,
@@ -80,6 +86,11 @@
 procutil,
 stringutil,
 )
+from hgext.show import (
+longestshortest,
+showview,
+)
+
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' 
for
 # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
@@ -462,6 +473,29 @@
 return result
 
 
+def getdrevmap(repo, revs):
+"""Return a dict mapping each rev in `revs` to their Differential
+Revision ID.
+"""
+result = {}
+for rev in revs:
+result[rev] = None
+ctx = repo[rev]
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+result[rev] = int(m.group('id'))
+continue
+# Check tags
+for tag in repo.nodetags(ctx.node()):
+m = _differentialrevisiontagre.match(tag)
+if m:
+result[rev] = int(m.group(1))
+break
+
+return result
+
+
 def getdiff(ctx, diffopts):
 """plain-text diff without header (user, commit message, etc)"""
 output = util.stringio()
@@ -1648,3 +1682,41 @@
 
 return templateutil.hybriddict({b'url': url, b'id': t,})
 return None
+
+
+@showview(b'phabstatus', csettopic=b'work')
+def phabstatusshowview(ui, repo, displayer):
+"""Phabricator differiential status"""
+revs = repo.revs('sort(_underway(), topo)')
+drevmap = getdrevmap(repo, revs)
+revs, drevids, revsbydrevid = [], set([]), {}
+for rev, drevid in pycompat.iteritems(drevmap):
+if drevid is not None:
+revs.append(rev)
+drevids.add(drevid)
+revsbydrevid.setdefault(drevid, set([])).add(rev)
+
+drevs = callconduit(ui, b'differential.query', {b'ids': list(drevids)})
+drevsbyrev = {}
+for drev in drevs:
+for rev in revsbydrevid[int(drev[b'id'])]:
+drevsbyrev[rev] = drev
+
+def phabstatus(ctx):
+drev = drevsbyrev[ctx.rev()]
+ui.write(b"\n%(uri)s %(statusName)s\n" % drev)
+
+revs = smartset.baseset(revs)
+revdag = graphmod.dagwalker(repo, revs)
+
+ui.setconfig(b'experimental', b'graphshorten', True)
+displayer._exthook = phabstatus
+nodelen = longestshortest(repo, revs)
+logcmdutil.displaygraph(
+ui,
+repo,
+revdag,
+displayer,
+graphmod.asciiedges,
+props={b'nodelen': nodelen},
+)



To: dlax, #hg-reviewers
Cc: mharbison72, mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax added a comment.
dlax planned changes to this revision.


  In D7506#110225 , @mharbison72 
wrote:
  
  > I like it.
  
  Thanks!
  
  > Any idea why the changeset isn't colored, unlike `hg show stack`?  It might 
also be a little more readable if the URI and status line were tabbed in, but 
maybe colored cset hashes would make that unnecessary?
  
  Yes, I messed up with adding a custom template whereas I can actually reuse 
"hg show"'s one. Will fix (along with another issue in the algorithm I just 
found out.)
  
  > I'm also interested in coloring the status value, but I can tinker with 
that after it's landed, unless you already have plans.
  
  You mean having a different color depending on status value, or a fixed one? 
I have no plan anyways, so if you have good ideas, I'll leave this up to you.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

To: dlax, #hg-reviewers
Cc: mharbison72, mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7508: relnotes: add note about changes to match.{explicit, reverse}dir

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> next:22
> +   also called when only `explicitdir` used to be called. That may
> +   mean that you can simple remove the use of `explicitdir` if you
> +   were already using `traversedir`.

simple -> simply?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7508/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7508

To: martinvonz, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7507: phabricator: add a "phabstatus" template keyword

[dlax (Denis Laxalde)]

dlax updated this revision to Diff 18316.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7507?vs=18314=18316

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7507/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7507

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1682,6 +1682,28 @@
 return None
 
 
+@eh.templatekeyword(b'phabstatus', requires={b'ctx', b'repo', b'ui'})
+def template_status(context, mapping):
+""":phabstatus: String. Status of Phabricator differential.
+"""
+ctx = context.resource(mapping, b'ctx')
+repo = context.resource(mapping, b'repo')
+ui = context.resource(mapping, b'ui')
+
+rev = ctx.rev()
+try:
+drevid = getdrevmap(repo, [rev])[rev]
+except KeyError:
+return None
+drevs = callconduit(ui, b'differential.query', {b'ids': [drevid]})
+for drev in drevs:
+if int(drev[b'id']) == drevid:
+return templateutil.hybriddict(
+{b'url': drev[b'uri'], b'status': drev[b'statusName'],}
+)
+return None
+
+
 phabstatus_tmpl = (
 b'{label("changeset.{phase}{if(troubles, \' changeset.troubled\')}", '
 b'shortest(node, 5))} '



To: dlax, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax updated this revision to Diff 18315.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7506?vs=18313=18315

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7506/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -11,6 +11,10 @@
 revisions in a format suitable for :hg:`import`, and a ``phabupdate`` command
 to update statuses in batch.
 
+A "phabstatus" view for :hg:`show` is also provided; it displays status
+information of Phabricator differentials associated with unfinished
+changesets.
+
 By default, Phabricator requires ``Test Plan`` which might prevent some
 changeset from being sent. The requirement could be disabled by changing
 ``differential.require-test-plan-field`` config server side.
@@ -60,9 +64,12 @@
 encoding,
 error,
 exthelper,
+formatter,
+graphmod,
 httpconnection as httpconnectionmod,
 match,
 mdiff,
+logcmdutil,
 obsutil,
 parser,
 patch,
@@ -80,6 +87,8 @@
 procutil,
 stringutil,
 )
+from hgext.show import showview
+
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' 
for
 # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
@@ -462,6 +471,29 @@
 return result
 
 
+def getdrevmap(repo, nodelist):
+"""Return a dict mapping each node in `nodelist` to their Differential
+Revision ID.
+"""
+result = {}
+for node in nodelist:
+result[node] = None
+ctx = repo[node]
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+result[node] = int(m.group('id'))
+continue
+# Check tags
+for tag in repo.nodetags(node):
+m = _differentialrevisiontagre.match(tag)
+if m:
+result[node] = int(m.group(1))
+break
+
+return result
+
+
 def getdiff(ctx, diffopts):
 """plain-text diff without header (user, commit message, etc)"""
 output = util.stringio()
@@ -1648,3 +1680,41 @@
 
 return templateutil.hybriddict({b'url': url, b'id': t,})
 return None
+
+
+phabstatus_tmpl = (
+b'{label("changeset.{phase}{if(troubles, \' changeset.troubled\')}", '
+b'shortest(node, 5))} '
+b'[{label("log.branch", branch)}] '
+b'{label("log.description", desc|firstline)} '
+b'({label("log.user", author|user)})\n'
+)
+
+
+@showview(b'phabstatus')
+def phabstatusshowview(ui, repo):
+"""Phabricator differiential status"""
+revs = repo.revs('sort(_underway(), topo)')
+drevmap = getdrevmap(repo, [repo[r].node() for r in revs])
+revs, drevids, revsbydrevid = [], [], {}
+for node, drevid in pycompat.iteritems(drevmap):
+if drevid is not None:
+revs.append(repo[node].rev())
+drevids.append(drevid)
+revsbydrevid[drevid] = repo[node].rev()
+
+revs = smartset.baseset(revs)
+drevs = callconduit(ui, b'differential.query', {b'ids': drevids})
+drevsbyrev = {revsbydrevid[int(drev[b'id'])]: drev for drev in drevs}
+
+def phabstatus(ctx):
+drev = drevsbyrev[ctx.rev()]
+ui.write(b"%(uri)s %(statusName)s\n" % drev)
+
+revdag = graphmod.dagwalker(repo, revs)
+
+ui.setconfig(b'experimental', b'graphshorten', True)
+spec = formatter.lookuptemplate(ui, None, phabstatus_tmpl)
+displayer = logcmdutil.changesettemplater(ui, repo, spec, buffered=True)
+displayer._exthook = phabstatus
+logcmdutil.displaygraph(ui, repo, revdag, displayer, graphmod.asciiedges)



To: dlax, #hg-reviewers
Cc: mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7507: phabricator: add a "phabstatus" template keyword

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We add a "phabstatus" template keyword, returning an object with "url"
  and "status" keys. This is quite similar to "phabreview" template
  keyword, but it queries phabricator for each specified revision so it's
  going to be slow (as compared to the "phabstatus" show view from
  previous changeset).

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7507

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1682,6 +1682,28 @@
 return None
 
 
+@eh.templatekeyword(b'phabstatus', requires={b'ctx', b'repo', b'ui'})
+def template_status(context, mapping):
+""":phabstatus: String. Status of Phabricator differential.
+"""
+ctx = context.resource(mapping, b'ctx')
+repo = context.resource(mapping, b'repo')
+ui = context.resource(mapping, b'ui')
+
+rev = ctx.rev()
+try:
+drevid = getdrevmap(repo, [rev])[rev]
+except KeyError:
+return None
+drevs = callconduit(ui, b'differential.query', {b'ids': [drevid]})
+for drev in drevs:
+if int(drev[b'id']) == drevid:
+return templateutil.hybriddict(
+{b'url': drev[b'uri'], b'status': drev[b'statusName'],}
+)
+return None
+
+
 phabstatus_tmpl = (
 b'{label("changeset.{phase}{if(troubles, \' changeset.troubled\')}", '
 b'shortest(node, 5))} '



To: dlax, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7506: phabricator: add a "phabstatus" show view

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added subscribers: mercurial-devel, Kwan, mjpieters.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We add a "phabstatus" show view (called as "hg show phabstatus") which
  renders a dag with underway revisions associated with a differential
  revision and displays their status.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7506

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -11,6 +11,10 @@
 revisions in a format suitable for :hg:`import`, and a ``phabupdate`` command
 to update statuses in batch.
 
+A "phabstatus" view for :hg:`show` is also provided; it displays status
+information of Phabricator differentials associated with unfinished
+changesets.
+
 By default, Phabricator requires ``Test Plan`` which might prevent some
 changeset from being sent. The requirement could be disabled by changing
 ``differential.require-test-plan-field`` config server side.
@@ -60,9 +64,12 @@
 encoding,
 error,
 exthelper,
+formatter,
+graphmod,
 httpconnection as httpconnectionmod,
 match,
 mdiff,
+logcmdutil,
 obsutil,
 parser,
 patch,
@@ -80,6 +87,8 @@
 procutil,
 stringutil,
 )
+from hgext.show import showview
+
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' 
for
 # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
@@ -462,6 +471,29 @@
 return result
 
 
+def getdrevmap(repo, nodelist):
+"""Return a dict mapping each node in `nodelist` to their Differential
+Revision ID (or None).
+"""
+result = {}
+for node in nodelist:
+result[node] = None
+ctx = repo[node]
+# Check commit message
+m = _differentialrevisiondescre.search(ctx.description())
+if m:
+result[node] = int(m.group('id'))
+continue
+# Check tags
+for tag in repo.nodetags(node):
+m = _differentialrevisiontagre.match(tag)
+if m:
+result[node] = int(m.group(1))
+break
+
+return result
+
+
 def getdiff(ctx, diffopts):
 """plain-text diff without header (user, commit message, etc)"""
 output = util.stringio()
@@ -1648,3 +1680,41 @@
 
 return templateutil.hybriddict({b'url': url, b'id': t,})
 return None
+
+
+phabstatus_tmpl = (
+b'{label("changeset.{phase}{if(troubles, \' changeset.troubled\')}", '
+b'shortest(node, 5))} '
+b'[{label("log.branch", branch)}] '
+b'{label("log.description", desc|firstline)} '
+b'({label("log.user", author|user)})\n'
+)
+
+
+@showview(b'phabstatus')
+def phabstatusshowview(ui, repo):
+"""Phabricator differiential status"""
+revs = repo.revs('sort(_underway(), topo)')
+drevmap = getdrevmap(repo, [repo[r].node() for r in revs])
+revs, drevids, revsbydrevid = [], [], {}
+for node, drevid in pycompat.iteritems(drevmap):
+if drevid is not None:
+revs.append(repo[node].rev())
+drevids.append(drevid)
+revsbydrevid[drevid] = repo[node].rev()
+
+revs = smartset.baseset(revs)
+drevs = callconduit(ui, b'differential.query', {b'ids': drevids})
+drevsbyrev = {revsbydrevid[int(drev[b'id'])]: drev for drev in drevs}
+
+def phabstatus(ctx):
+drev = drevsbyrev[ctx.rev()]
+ui.write(b"%(uri)s %(statusName)s\n" % drev)
+
+revdag = graphmod.dagwalker(repo, revs)
+
+ui.setconfig(b'experimental', b'graphshorten', True)
+spec = formatter.lookuptemplate(ui, None, phabstatus_tmpl)
+displayer = logcmdutil.changesettemplater(ui, repo, spec, buffered=True)
+displayer._exthook = phabstatus
+logcmdutil.displaygraph(ui, repo, revdag, displayer, graphmod.asciiedges)



To: dlax, #hg-reviewers
Cc: mjpieters, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7505: logcmdutil: call _exthook() in changesettemplater

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Class changesetprinter has an _exthook() method that is called in
  _show() before the patch is displayed. Call the method as well in
  changesettemplater._show().

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7505

AFFECTED FILES
  mercurial/logcmdutil.py

CHANGE DETAILS

diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -597,6 +597,7 @@
 # write changeset metadata, then patch if requested
 key = self._parts[self._tref]
 self.ui.write(self.t.render(key, props))
+self._exthook(ctx)
 self._showpatch(ctx, graphwidth)
 
 if self._parts[b'footer']:



To: dlax, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7504: py3: replace %s by %r on binary format string when needed

[dlax (Denis Laxalde)]

This revision now requires changes to proceed.
dlax added a comment.
dlax requested changes to this revision.


  nit: the actual PEP is pep-0461 (https://www.python.org/dev/peps/pep-0461/)

INLINE COMMENTS

> localrepo.py:1571
> +b"unsupported changeid '%r' of type %r"
>  % (changeid, pycompat.sysstr(type(changeid)))
>  )

The first `%s` was correct I think because `changeid` can be a bytes.

For the second one, if `%r` is the way to go (I'm not sure), maybe we can drop 
`pycompat.sysstr()`?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7504/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7504

To: matclab, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7051: phabricator: remove tests and all recordings

[dlax (Denis Laxalde)]

dlax added a comment.


  > The next commit is going to change the format of conduit API requests so 
none of the VCR recordings will match and all the tests will fail.
  
  @Kwan, it seems to me that there is no longer any recording data nor actual 
test from this changeset. Did you plan to add them back? (At the moment, it 
seems to me that test-phabricator.t tests nothing.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7051/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7051

To: Kwan, #hg-reviewers
Cc: dlax, mjpieters, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7460: tests: add more tests for "hg shelve --delete"

[dlax (Denis Laxalde)]

Closed by commit rHG4330851947fb: tests: add more tests for hg shelve 
--delete (authored by dlax).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7460?vs=18250=18258

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7460/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7460

AFFECTED FILES
  tests/test-shelve.t

CHANGE DETAILS

diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -951,6 +951,16 @@
   +++ b/jungle
   @@ -0,0 +1,1 @@
   +babar
+
+Test shelve --delete
+
+  $ hg shelve --list
+  default (*s ago)changes to: create conflict (glob)
+  $ hg shelve --delete doesnotexist
+  abort: shelved change 'doesnotexist' not found
+  [255]
+  $ hg shelve --delete default
+
   $ cd ..
 
 Test visibility of in-memory changes inside transaction to external hook



To: dlax, #hg-reviewers, mharbison72, pulkit
Cc: mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7465: filemerge: fix a missing attribute usage

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> mharbison72 wrote in filemerge.py:122
> I’m not sure how this works, but this was enough to make pytype happy.  Does 
> it make any sense to compare an absent file to an absent file?
> 
> Maybe the right thing is to add a ctx attribute here?

> Does it make any sense to compare an absent file to an absent file?

Not sure, but due to lazy evaluation `fctx.ctx() == self.ctx()` would only be 
evaluated if `fctx` is an absent file, hence also lacking the `ctx()` method.
I suggested to use `changectx()` because this is already implemented and also 
part of the basefilectx interface.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7465/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7465

To: mharbison72, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7465: filemerge: fix a missing attribute usage

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> filemerge.py:122
>  fctx.isabsent()
> -and fctx.ctx() == self.ctx()
> +and fctx.ctx() == self._ctx
>  and fctx.path() == self.path()

What about `fctx`? It could also lack the `ctx()` method if an instance of 
`absentfilectx`.
I wonder if the intent was not `fctx.changectx() == self.changectx()`.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7465/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7465

To: mharbison72, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7464: filemerge: drop a default argument to appease pytype

[dlax (Denis Laxalde)]

dlax added a comment.


  Shouldn't this be also done for all similar functions? (i.e. `_xmergeimm` and 
functions registered as a merge tool with `@internaltool`)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7464/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7464

To: mharbison72, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7460: tests: add more tests for "hg shelve --delete"

[dlax (Denis Laxalde)]

dlax updated this revision to Diff 18250.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7460?vs=18249=18250

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7460/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7460

AFFECTED FILES
  tests/test-shelve.t

CHANGE DETAILS

diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -951,6 +951,16 @@
   +++ b/jungle
   @@ -0,0 +1,1 @@
   +babar
+
+Test shelve --delete
+
+  $ hg shelve --list
+  default (*s ago)changes to: create conflict (glob)
+  $ hg shelve --delete doesnotexist
+  abort: shelved change 'doesnotexist' not found
+  [255]
+  $ hg shelve --delete default
+
   $ cd ..
 
 Test visibility of in-memory changes inside transaction to external hook



To: dlax, #hg-reviewers
Cc: mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7458: shelve: add the missing `create` parameter to the bundlerepo constructor

[dlax (Denis Laxalde)]

dlax added a comment.
dlax accepted this revision.


  Good catch.
  
  On the other hand, it's not clear to me what's the point of this "create" 
argument given `bundlerepo.instance()` will just use it to raise Abort if it is 
true.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7458/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7458

To: mharbison72, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7460: tests: add more tests for "hg shelve --delete"

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  It appears that the only tests for "hg shelve --delete" concern command
  errors (e.g. incompatible command options). Adding some more to check
  that non-existent names are handled and a success case.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7460

AFFECTED FILES
  tests/test-shelve.t

CHANGE DETAILS

diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -951,6 +951,16 @@
   +++ b/jungle
   @@ -0,0 +1,1 @@
   +babar
+
+Test shelve --delete
+
+  $ hg shelve --list
+  default (1s ago)changes to: create conflict
+  $ hg shelve --delete doesnotexist
+  abort: shelved change 'doesnotexist' not found
+  [255]
+  $ hg shelve --delete default
+
   $ cd ..
 
 Test visibility of in-memory changes inside transaction to external hook



To: dlax, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7296: pycompat: kludge around pytype being confused by __new__

[dlax (Denis Laxalde)]

dlax added a comment.


  In D7296#109684 , @dlax wrote:
  
  > Looking closer at the error above, it mentions `bytestr.__init__`, not 
`__new__` (and there is in fact no type annotation for `__new__` in typeshed 
).
  
  https://github.com/python/typeshed/issues/2630

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg, dlax
Cc: yuja, mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7296: pycompat: kludge around pytype being confused by __new__

[dlax (Denis Laxalde)]

dlax added a comment.
dlax added a subscriber: yuja.


  In D7296#109683 , @durin42 wrote:
  
  > In D7296#109672 , @dlax wrote:
  >
  >> Sorry, still not ok afaict :/
  >
  > So, I tried fixing this and it actually made things worse? dagparser.py no 
longer typechecks if I correct the syntax? Try the pytype invocation from the 
test at the end of the series and you'll see what I mean.
  
  Ok, trying `pytype mercurial/dagparser.py` I get a couple of those errors:
  
File ".../mercurial/dagparser.py", line 172, in parsedag: Function 
bytestr.__init__ was called with the wrong arguments [wrong-arg-types]
  Expected: (self, ints: Iterable[int])
  Actually passed: (self, ints: str)
  
  My point was that we need to keep "mercurial/pycompat.py" passing pytype 
before considering modules it depends on. Once the missing `type: ` is added 
and bytestr <-> _bytestr trick applied, it's okay but the error in dagparser.py 
persists...
  Looking closer at the error above, it mentions `bytestr.__init__`, not 
`__new__` (and there is in fact no type annotation for `__new__` in typeshed 
).
 So I suspect the "Callable" trick is not enough and we'd need a workaround 
similar to da925257 
 by 
@yuja .

INLINE COMMENTS

> pycompat.py:305
>  
> +bytestr = _bytestr  # Callable[[Union[bytes, str]], bytestr]
> +

Now the `type: ` is missing, so the comment is ignored.

Adding it, `pytype mercurial/pycompat.py` gives:

  mercurial/pycompat.py", line 305, in : Invalid type comment: 
Callable[[Union[bytes, str]], bytestr] [invalid-type-comment]
Name 'bytestr' is not defined

hence the kind of trick I suggested in my first comment on this line.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg, dlax
Cc: yuja, mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7296: pycompat: kludge around pytype being confused by __new__

[dlax (Denis Laxalde)]

This revision now requires changes to proceed.
dlax added a comment.
dlax requested changes to this revision.


  Sorry, still not ok afaict :/

INLINE COMMENTS

> pycompat.py:305
>  
> +bytestr = _bytestr  # type: Callable[[Union[bytes, str], bytestr]]
> +

`]` is still at the wrong place, I think. Should be `Callable[[Union[bytes, 
str]], bytestr]`.
Then I still get the "Name 'bytestr' is not defined" error mentioned above.

> pycompat.py:414
>  byterepr = repr
> -bytestr = str
> +bytestr = str  # type: Callable[[Union[bytes, str], bytestr]
>  iterbytestr = iter

same here about missing `]`.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg, dlax
Cc: mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7408: extensions: hide two confusing import statements from pytype

[dlax (Denis Laxalde)]

dlax added a comment.


  In D7408#109565 , @indygreg 
wrote:
  
  > Where does `hgext.__index__` come from?!
  
  This is generated by setup.py, apparently only when building with py2exe.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7408/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7408

To: durin42, #hg-reviewers, dlax, indygreg
Cc: indygreg, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7296: pycompat: kludge around pytype being confused by __new__

[dlax (Denis Laxalde)]

This revision now requires changes to proceed.
dlax added inline comments.
dlax requested changes to this revision.

INLINE COMMENTS

> pycompat.py:294
>  
> +bytestr = _bytestr  # type: Callable[[Union[bytes, str], bytestr]
> +

A `]` is missing before `, bytestr`. Then, it's also missing `typing` imports.
But, even with this, I get:

  Invalid type comment: Callable[[Union[bytes, str]], bytestr] 
[invalid-type-comment]
Name 'bytestr' is not defined

I'm able to make this pass with:

  bytestr = _bytestr
  bytestr = bytestr  # type: Callable[[Union[bytes, str]], bytestr]

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg, dlax
Cc: mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7410: extensions: suppress a pytype failure due to a typeshed bug

[dlax (Denis Laxalde)]

dlax added a comment.
dlax accepted this revision.


  meanwhile, https://github.com/python/typeshed/pull/3465

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7410/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7410

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7296: pycompat: kludge around pytype being confused by __new__

[dlax (Denis Laxalde)]

dlax added a comment.


  black complains because inline comments have only one space before (esp. the 
first one produces a parse error).
  LGTM otherwise.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg, dlax
Cc: mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7384: commands: necessary annotations and suppresssions to pass pytype

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> commands.py:4742
> +# warning about list not having a max() method.
> +endrev = revs.max() + 1  # pytype: disable=attribute-error
>  getcopies = scmutil.getcopiesfn(repo, endrev=endrev)

`revs` is always a `smartset.baseset` per af9c73f26371 
 so 
there should be no attribute error. Or is it because 
`logcmdutil.getlinerangerevs()` has no type annotation (whereas 
`logcmdutil.getrevs()` has some)?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7430: bisect: replace try:/finally: by a "restore_state" context manager

[dlax (Denis Laxalde)]

Closed by commit rHGf37da59a36d9: bisect: replace try:/finally: by a 
restore_state context manager (authored by dlax).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs 
Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7430?vs=18156=18160

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7430/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7430

AFFECTED FILES
  mercurial/commands.py
  mercurial/hbisect.py

CHANGE DETAILS

diff --git a/mercurial/hbisect.py b/mercurial/hbisect.py
--- a/mercurial/hbisect.py
+++ b/mercurial/hbisect.py
@@ -11,6 +11,7 @@
 from __future__ import absolute_import
 
 import collections
+import contextlib
 
 from .i18n import _
 from .node import (
@@ -180,6 +181,15 @@
 raise error.Abort(_(b'cannot bisect (no known bad revisions)'))
 
 
+@contextlib.contextmanager
+def restore_state(repo, state, node):
+try:
+yield
+finally:
+state[b'current'] = [node]
+save_state(repo, state)
+
+
 def get(repo, status):
 """
 Return a list of revision(s) that match the given status:
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1073,7 +1073,7 @@
 raise error.Abort(_(b'current bisect revision is a merge'))
 if rev:
 node = repo[scmutil.revsingle(repo, rev, node)].node()
-try:
+with hbisect.restore_state(repo, state, node):
 while changesets:
 # update state
 state[b'current'] = [node]
@@ -1105,9 +1105,6 @@
 # update to next check
 node = nodes[0]
 mayupdate(repo, node, show_stats=False)
-finally:
-state[b'current'] = [node]
-hbisect.save_state(repo, state)
 hbisect.printresult(ui, repo, state, displayer, nodes, bgood)
 return
 



To: dlax, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7410: extensions: suppress a strange pytype failure

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> dlax wrote in extensions.py:96
> the doc indeed says that "fd" is file object

In https://github.com/python/typeshed/blob/master/stdlib/3/imp.pyi type of 
`find_module` seems wrong.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7410/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7410

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7410: extensions: suppress a strange pytype failure

[dlax (Denis Laxalde)]

dlax added inline comments.
dlax accepted this revision.

INLINE COMMENTS

> extensions.py:96
> +# pytype seems to think `fd` is a str, but I'm pretty sure
> +# it's wrong. This may be a bug we need to report upstream.
> +return imp.load_module(

the doc indeed says that "fd" is file object

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7410/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7410

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7408: extensions: hide two confusing import statements from pytype

[dlax (Denis Laxalde)]

dlax added a comment.
dlax accepted this revision.


  Out of curiosity, where does this `__index__` value come from?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7408/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7408

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7384: commands: necessary annotations and suppresssions to pass pytype

[dlax (Denis Laxalde)]

This revision now requires changes to proceed.
dlax added a comment.
dlax requested changes to this revision.


  D7430  makes this changes unnecessary I 
think.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7296: pycompat: kludge around pytype being confused by __new__

[dlax (Denis Laxalde)]

This revision now requires changes to proceed.
dlax added a comment.
dlax requested changes to this revision.


  Rather `class bytestr(bytes):  # type: Callable[[Union[bytes, str], bytestr]` 
as @yuya suggested in D7380 .

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg, dlax
Cc: mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7430: bisect: replace try:/finally: by a "restore_state" context manager

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This should help pytype to not consider "bgood" variable as NameError.
  See https://phab.mercurial-scm.org/D7384 for context.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7430

AFFECTED FILES
  mercurial/commands.py
  mercurial/hbisect.py

CHANGE DETAILS

diff --git a/mercurial/hbisect.py b/mercurial/hbisect.py
--- a/mercurial/hbisect.py
+++ b/mercurial/hbisect.py
@@ -11,6 +11,7 @@
 from __future__ import absolute_import
 
 import collections
+import contextlib
 
 from .i18n import _
 from .node import (
@@ -180,6 +181,15 @@
 raise error.Abort(_(b'cannot bisect (no known bad revisions)'))
 
 
+@contextlib.contextmanager
+def restore_state(repo, state, node):
+try:
+yield
+finally:
+state[b'current'] = [node]
+save_state(repo, state)
+
+
 def get(repo, status):
 """
 Return a list of revision(s) that match the given status:
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1073,7 +1073,7 @@
 raise error.Abort(_(b'current bisect revision is a merge'))
 if rev:
 node = repo[scmutil.revsingle(repo, rev, node)].node()
-try:
+with hbisect.restore_state(repo, state, node):
 while changesets:
 # update state
 state[b'current'] = [node]
@@ -1105,9 +1105,6 @@
 # update to next check
 node = nodes[0]
 mayupdate(repo, node, show_stats=False)
-finally:
-state[b'current'] = [node]
-hbisect.save_state(repo, state)
 hbisect.printresult(ui, repo, state, displayer, nodes, bgood)
 return
 



To: dlax, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7296: pycompat: kludge around pytype being confused by __new__

[dlax (Denis Laxalde)]

dlax added a comment.


class bytestr(bytes):  # type: (Union[bytes,str]) -> bytestr
  [...]
  
  Does this work?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg
Cc: mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7380: encoding: define per-use identity functions only in typechecking mode

[dlax (Denis Laxalde)]

dlax added a comment.


  Have you tried using variables annotations? Like:
  
strtolocal = pycompat.identity  # type: (str) -> bytes
strfromlocal = pycompat.identity  # type: (bytes) -> str

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7380/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7380

To: durin42, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7385: debugcommands: suppress import errors for pytype

[dlax (Denis Laxalde)]

dlax added a comment.
dlax accepted this revision.


  I wonder if using `importlib.import_module()` wouldn't help. Or is it 
something we avoid in Mercurial?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7385/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7385

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7384: commands: necessary annotations and suppresssions to pass pytype

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> commands.py:1127
> +bgood,  # pytype: disable=name-error
> +)
>  return

This one is sad. I think this can be sorted out by replacing the 
`try:/finally:` with a context manager. (Can send a patch, if it sounds good to 
you.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7289: branchmap: always copy closednodes to a set

[dlax (Denis Laxalde)]

dlax added a comment.


  On second thought, it's not obvious why it'd be better than annotating 
`__init__()`. Is this because this would require many changes in callers?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7289/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7289

To: durin42, #hg-reviewers, indygreg, dlax
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7381: cmdutil: add a pytype annotation to help out some callsites

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> cmdutil.py:3970
>  def readgraftstate(repo, graftstate):
> +# type: (Any, statemod.cmdstate) -> Dict[bytes, Any]
>  """read the graft state file and return a dict of the data stored in 
> it"""

Wouldn't `-> Dict[bytes, List[bytes]]` be okay? (Not sure why "Any" you're 
referering to in commit message, though I understand `Dict[bytes, Any]` comes 
from `state.cmdstate.read()` return type in D7383 
.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7381/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7381

To: durin42, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7377: commands: log --line-range is incompatible with --copies

[dlax (Denis Laxalde)]

dlax added a comment.


  Nice pytype catch!
  
  That's a bug, I think. `logcmdutil.getlinerangerevs()` should return `revs` 
as a smartset, not as list. I'll work on a fix.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7377/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7377

To: durin42, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7373: py3: pass a bytes value for "msg" to nouideprecwarn()

[dlax (Denis Laxalde)]

Closed by commit rHGc207c46a86b9: py3: pass a bytes value for msg 
to nouideprecwarn() (authored by dlax).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7373?vs=18051=18053

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7373/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7373

AFFECTED FILES
  mercurial/pure/parsers.py
  mercurial/revlog.py

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -207,7 +207,7 @@
 class revlogoldindex(list):
 @property
 def nodemap(self):
-msg = "index.nodemap is deprecated, " "use 
index.[has_node|rev|get_rev]"
+msg = b"index.nodemap is deprecated, use index.[has_node|rev|get_rev]"
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self._nodemap
 
@@ -657,15 +657,15 @@
 @property
 def nodemap(self):
 msg = (
-"revlog.nodemap is deprecated, "
-"use revlog.index.[has_node|rev|get_rev]"
+b"revlog.nodemap is deprecated, "
+b"use revlog.index.[has_node|rev|get_rev]"
 )
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self.index.nodemap
 
 @property
 def _nodecache(self):
-msg = "revlog._nodecache is deprecated, use revlog.index.nodemap"
+msg = b"revlog._nodecache is deprecated, use revlog.index.nodemap"
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self.index.nodemap
 
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -49,7 +49,7 @@
 class BaseIndexObject(object):
 @property
 def nodemap(self):
-msg = "index.nodemap is deprecated, " "use 
index.[has_node|rev|get_rev]"
+msg = b"index.nodemap is deprecated, use index.[has_node|rev|get_rev]"
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self._nodemap
 



To: dlax, indygreg, #hg-reviewers, pulkit
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7373: py3: pass a bytes value for "msg" to nouideprecwarn()

[dlax (Denis Laxalde)]

dlax added a comment.


  should fix tracebacks in https://ci.octobus.net/job/EvolvePy3/278/console

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7373/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7373

To: dlax, indygreg, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7373: py3: pass a bytes value for "msg" to nouideprecwarn()

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added a reviewer: indygreg.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  That function formats "msg" with the "version" value. On Python 3, this
  leads to "TypeError: can only concatenate str (not "bytes") to str".
  
  Also eliminate spurious strings concatenation in single-line
  declarations.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7373

AFFECTED FILES
  mercurial/pure/parsers.py
  mercurial/revlog.py

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -207,7 +207,7 @@
 class revlogoldindex(list):
 @property
 def nodemap(self):
-msg = "index.nodemap is deprecated, " "use 
index.[has_node|rev|get_rev]"
+msg = b"index.nodemap is deprecated, use index.[has_node|rev|get_rev]"
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self._nodemap
 
@@ -657,15 +657,15 @@
 @property
 def nodemap(self):
 msg = (
-"revlog.nodemap is deprecated, "
-"use revlog.index.[has_node|rev|get_rev]"
+b"revlog.nodemap is deprecated, "
+b"use revlog.index.[has_node|rev|get_rev]"
 )
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self.index.nodemap
 
 @property
 def _nodecache(self):
-msg = "revlog._nodecache is deprecated, use revlog.index.nodemap"
+msg = b"revlog._nodecache is deprecated, use revlog.index.nodemap"
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self.index.nodemap
 
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -49,7 +49,7 @@
 class BaseIndexObject(object):
 @property
 def nodemap(self):
-msg = "index.nodemap is deprecated, " "use 
index.[has_node|rev|get_rev]"
+msg = b"index.nodemap is deprecated, use index.[has_node|rev|get_rev]"
 util.nouideprecwarn(msg, b'5.3', stacklevel=2)
 return self._nodemap
 



To: dlax, indygreg, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7262: templateutil: fix a missing ABCMeta assignment

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> dlax wrote in templateutil.py:114
> This is ignored on Python 3 (meaning it's possible to instantiate `mappable` 
> directly, despite some methods being abstract). It should be `class 
> mappable(metaclass=abc.ABCMeta)` on Python 3. 
> But I realize that there are other `__metaclass__ = abc.ABCMeta` in the code 
> base. Do we handle this somehow or has this been overlooked?

Just seen D7272  and others which explain 
this. Nevermind.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7262/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7262

To: durin42, #hg-reviewers, indygreg
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7262: templateutil: fix a missing ABCMeta assignment

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> templateutil.py:114
>  
> +__metaclass__ = abc.ABCMeta
> +

This is ignored on Python 3 (meaning it's possible to instantiate `mappable` 
directly, despite some methods being abstract). It should be `class 
mappable(metaclass=abc.ABCMeta)` on Python 3. 
But I realize that there are other `__metaclass__ = abc.ABCMeta` in the code 
base. Do we handle this somehow or has this been overlooked?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7262/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7262

To: durin42, #hg-reviewers, indygreg
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2282: util: extract all date-related utils in utils/dateutil module

[dlax (Denis Laxalde)]

dlax added a comment.


  lothiraldan (Boris Feld) wrote:
  
  > I have but I'm always very cautious when creating a new package with the
  >  name of an old module. .pyc/.pycache files may still be there both for
  >  Mercurial developers and for Mercurial users using their deb/rpm package.
  > 
  > I may be wrong, but if we could avoid weird bugs, I would prefer put in
  >  the utils package.
  
  Well, this is Python... Most people are aware of this kind of issues, I
  think. So the argument sounds a bit like FUD :)
  
  On the other hand, if we keep going that route, we'll have to live with
  util/utils "forever" just to have avoided this hypothetical
  inconvenience, is this more acceptable?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2282

To: lothiraldan, #hg-reviewers
Cc: dlax, martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2280: remotenames: port partway to python3

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> remotenames.py:30
> +import collections
> +dictmixin = collections.MutableMapping
>  

`collections.MutableMapping` exists on Python2 as well (from 2.6 apparently), 
can't we use it?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2280

To: durin42, #hg-reviewers
Cc: dlax, yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2282: util: extract all date-related utils in utils/dateutil module

[dlax (Denis Laxalde)]

dlax added a comment.


  Having both a `util` module and a `utils` package looks weird.
  Have you considered moving `util.py` into `util/__init__.py` and then adding 
new modules under `util` package?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2282

To: lothiraldan, #hg-reviewers
Cc: dlax, martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2095: clone: updates the help text for hg clone -r (issue5654) [bugzilla] and hg clone -b

[dlax (Denis Laxalde)]

dlax added a comment.


  sangeet259 (Sangeet Kumar Mishra) wrote:
  
  > @dlax https://phab.mercurial-scm.org/p/dlax/ Yes, but the short 
  >  summary didn't say what it does! It just says "include the specified 
  >  changeset".
  
  I agree it's not super-clear. Perhaps we could say "clone only specified 
  changeset" for -r (help for -b looks ok to me).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2095

To: sangeet259, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2095: clone: updates the help text for hg clone -r (issue5654) [bugzilla] and hg clone -b

[dlax (Denis Laxalde)]

dlax added a comment.


  As mentioned in the issue, there's already an explanation paragraph help:
  
To pull only a subset of changesets, specify one or more revisions
identifiers with -r/--rev or branches with -b/--branch. The resulting
clone will contain only the specified changesets and their ancestors.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2095

To: sangeet259, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1616: rebase: disable `inmemory` if the rebaseset contains the working copy

[dlax (Denis Laxalde)]

dlax added a comment.


  In https://phab.mercurial-scm.org/D1616#27819, @durin42 wrote:
  
  > @dlax I see a request changes here, but I don't see any commentary as to 
why?
  
  
  I meant to comment that the first hunks (i.e.  those above the big comment 
block) seem unrelated and to ask for clarification.
  No idea where my comment went, sorry...

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1616

To: phillco, #hg-reviewers, dlax
Cc: durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1249: rebase: rerun a rebase on-disk if IMM merge conflicts arise

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> durin42 wrote in rebase.py:667
> Avoiding the recursion seems like a nice thing to me. I  prefer what Phil has 
> to just doing recursion...

Why would that be nice? Do you foresee any problem?
It just makes the code harder to follow, IMHO.

Besides, now the `rebase()` no longer has a docstring, meaning that help is 
broken.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1249

To: phillco, #hg-reviewers, durin42, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1232: rebase: add the --inmemory option flag; assign a wctx object for the rebase

[dlax (Denis Laxalde)]

dlax added a comment.


  dlax (Denis Laxalde) wrote:
  
  >   Also, would be nice to have some test coverage.
  
  Unless I missed it, there's still no test at all for the --inmemory
  option in this series.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1232

To: phillco, #hg-reviewers, dlax, durin42
Cc: durin42, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1249: rebase: rerun a rebase on-disk if IMM merge conflicts arise

[dlax (Denis Laxalde)]

dlax requested changes to this revision.
dlax added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> rebase.py:667
> +
> +def _origrebase(ui, repo, **opts):
>  """move changeset (and descendants) to a different branch

Do we need to make another function? Can't we call rebase again with options 
modified?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1249

To: phillco, #hg-reviewers, durin42, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1245: rebase: pass wctx to rebasenode()

[dlax (Denis Laxalde)]

dlax requested changes to this revision.
dlax added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> rebase.py:995
>  
> -def rebasenode(repo, rev, p1, base, state, collapse, dest):
> +def rebasenode(repo, rev, p1, base, state, collapse, dest, wctx=None):
>  'Rebase a single revision rev on top of p1 using base as merge ancestor'

Isn't it meant to be used in the function body? I see a `wctx = repo[None]` 
below, perhaps it should be dropped.

But this wouldn't work with a `None` default value as mentioned in the next 
patch.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1245

To: phillco, #hg-reviewers, durin42, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1246: rebase: do not update if IMM; instead, set the overlaywctx's parents

[dlax (Denis Laxalde)]

dlax requested changes to this revision.
dlax added a comment.
This revision now requires changes to proceed.


  Needs test.

INLINE COMMENTS

> rebase.py:1003
>  # Update to destination and merge it with local
> -if repo['.'].rev() != p1:
> -repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1]))
> -mergemod.update(repo, p1, False, True)
> +if wctx.isinmemory():
> +wctx.setbase(repo[p1])

`wctx` has a default value of `None`, so either it shouldn't have that or there 
should be a check for `None` here.

> rebase.py:1007
> +# This is necessary to invalidate workingctx's caches.
> +wctx = repo[None]
> +if repo['.'].rev() != p1:

`wctx` is redefined here.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1246

To: phillco, #hg-reviewers, durin42, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1244: overlayworkingctx: invalidate the manifest cache when changing parents

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> context.py:2003
> +# Drop old manifest cache:
> +self._invalidate()
>  

Wouldn't `util.clearcachedproperty(self, '_manifest')` work?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1244

To: phillco, #hg-reviewers, durin42
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1232: rebase: add the --inmemory option flag; assign a wctx object for the rebase

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> rebase.py:738
> +if 'inmemory' not in opts:
> +opts['inmemory'] = False
>  rbsrt = rebaseruntime(repo, ui, opts)

This is useless because "inmemory" will be in `opts` and because you wrote 
`opts.get('inmemory', False)` in `rebaseruntime.__init__()` above.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1232

To: phillco, #hg-reviewers, dlax, durin42
Cc: durin42, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1232: rebase: add the --inmemory option flag; assign a wctx object for the rebase

[dlax (Denis Laxalde)]

dlax requested changes to this revision.
dlax added a comment.
This revision now requires changes to proceed.


  > In the future, the --inmemory flag might be deprecated in favor of 
something more intelligent
  
  Perhaps the option should be flagged experimental then?
  
  Also, would be nice to have some test coverage.

INLINE COMMENTS

> rebase.py:727
> +if 'inmemory' not in opts:
> +opts['inmemory'] = False
>  rbsrt = rebaseruntime(repo, ui, opts)

Not sure this is needed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1232

To: phillco, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D939: remotenames: add functionality to store remotenames under .hg/hgremotenames/

[dlax (Denis Laxalde)]

dlax added a comment.


  pulkit (Pulkit Goyal) a écrit :
  
  > Updated the series with new differentials 
https://phab.mercurial-scm.org/D1547
  >  https://phab.mercurial-scm.org/D1547 to 
https://phab.mercurial-scm.org/D1551
  >  https://phab.mercurial-scm.org/D1551.
  
  Why creating new differentials? I thought the very point of Phabricator
  was to make it possible to review several versions of a given patch on
  the same entity. By abandoning and creating new differentials, we loose
  previous review feedback and it breaks the subscription group (which,
  given how handy it is subscribe to a series of differentials, is not
  really nice).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D939

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: durin42, ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1502: rewriteutil: add utility function to check if we can create new unstable cset

[dlax (Denis Laxalde)]

dlax requested changes to this revision.
dlax added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> rewriteutil.py:20
> +To allow new unstable changesets, set the config:
> +`experimental.evolution.allounstable=True`
> +"""

typo: allounstable

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1502

To: pulkit, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1502: rewriteutil: add utility function to check if we can create new unstable cset

[dlax (Denis Laxalde)]

dlax added a comment.


  > This rewriteutil.py introduced in this patch and the utility functions 
added in the upcoming patches exists in the evolve extension are being ported 
from there.
  
  Is it worth porting this alone if nothing in core makes use of this module?
  Or is there a larger plan?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1502

To: pulkit, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1483: globalopts: make special flags ineffective after '--' (BC)

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> dispatch.py:715
> +return True
> +return False
> +

I find the algorithm a bit clumsy (usage of both `in` and `.index()` for the 
same value), how about a for loop like that:

  for arg in args:
  if arg == optname:
  return True
  if arg == '--':
  return False
  return False

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1483

To: quark, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1358: remotenames: store journal entry for bookmarks if journal is loaded

[dlax (Denis Laxalde)]

dlax added a comment.


  The state of this stack is not quite clear: there are abandoned revisions and 
the first changeset (introducing "mercurial/remotenames.py" file) seems to be 
missing.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1358

To: pulkit, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1371: global: remove redundant parenthesis

[dlax (Denis Laxalde)]

dlax added a comment.


  LGTM modulo a few nits. Nice cleanup.

INLINE COMMENTS

> hgk.py:79
>  ('S', 'search', "", _('search'))],
> -('[OPTION]... NODE1 NODE2 [FILE]...'),
> + '[OPTION]... NODE1 NODE2 [FILE]...',
>  inferrepo=True)

Why this extra indentation?

> hgk.py:337
>  ('n', 'max-count', 0, _('max-count'))],
> -('[OPTION]... REV...'))
> + '[OPTION]... REV...')
>  def revlist(ui, repo, *revs, **opts):

Spurious indentation changes here as well.

> bookmarks.py:319
>  parents = [p.node() for p in repo[None].parents()]
> -return (mark in marks and marks[mark] in parents)
> +return mark in marks and marks[mark] in parents
>  

I'd keep this one as it makes it clearer that we return a generator. Arguably, 
that's nitpicking.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1371

To: indygreg, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1378: bundlerepo: assign bundle attributes in bundle type blocks

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> bundlerepo.py:298
>  
> -self._handlebundle2part(part)
> +self._handlebundle2part(bundle, part)
>  

Within `_handlebundle2part`, there's still a `self._bundle = 
changegroup.getunbundler(...)` statement. Perhaps, it'd be even clearer if this 
method would `return changegroup.getunbundler(...)` and let the caller 
eventually assign this as an attribute.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1378

To: indygreg, #hg-reviewers, dlax
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1372: bundlerepo: make methods agree with base class

[dlax (Denis Laxalde)]

dlax added a comment.


  > For methods that are implemented, we change arguments to match the base.
  
  Alternatively, we could use `**kwargs` for keywords arguments unused in a 
method. I think that's a common pattern and it avoids confusing the reader with 
unused arguments.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1372

To: indygreg, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1348: histedit: add support to output nodechanges using formatter

[dlax (Denis Laxalde)]

dlax requested changes to this revision.
dlax added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> histedit.py:920
> + ('r', 'rev', [], _('first revision to be edited'), _('REV'))] +
> +  cmdutil.templateopts,
>   _("[OPTIONS] ([ANCESTOR] | --outgoing [URL])"))

`formatteropts` should be enought (you don't need `--style` I guess)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1348

To: pulkit, durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1270: help: adding a topic on flags

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> flags.txt:30
> +In order to set a default value for a flag in an hgrc file, set it under the
> +[defaults] section of the hgrc file::
> +

`hg help config` says that "defaults" are deprecated and that aliases should be 
used instead.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1270

To: rdamazio, #hg-reviewers
Cc: dlax, martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1173: rebase: add support to output nodechanges

[dlax (Denis Laxalde)]

dlax accepted this revision.
dlax added a comment.


  Looks good to me, much nicer than the previous version.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1173

To: pulkit, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1063: rebase: enable multidest by default

[dlax (Denis Laxalde)]

dlax added a comment.


  In https://phab.mercurial-scm.org/D1063#18725, @durin42 wrote:
  
  > (Note that I'd still welcome feedback from non-BigCo contributors here - is 
this something we should make permanent? Have people been testing this? Etc.)
  
  
  I have never tested this but it seems to me that this option enables a rather 
advanced behavior that would get rarely used (I cannot think of a situation 
where I'd need this FWIW). So having it on by default seems wrong to me.
  On the other hand, as said in the commit message, you already have to used 
//advanced// names `SRC` or `ALLSRC` to see the actual effect of this option, 
so maybe it's already fine. Ultimately, if it's really safe, why keeping the 
option at all?
  
  Anyways, I'd also welcome more feedback from people already using it, perhaps 
other people from FB could share their experience here?

INLINE COMMENTS

> rebase.py:645
> +``ALLSRC`` substituted by all source revisions.
> +
>  Rebase will destroy original changesets unless you use ``--keep``.

Maybe this could be in a `.. container:: verbose` block since it's an advanced 
feature?

> rebase.py:698
> + -d 'first(max((successors(max(roots(ALLSRC) & ::SRC)^)-obsolete())::) +\
> + max(::((roots(ALLSRC) & ::SRC)^)-obsolete()))'
> +

I'd also be very useful to have an example of how to stabilize only the current 
stack, but maybe this is unrelated to this changeset.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1063

To: quark, #hg-reviewers, durin42
Cc: dlax, martinvonz, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1074: branch: add a --rev flag to change branch name of given revisions

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> cmdutil.py:749
> +if len(heads) < 1:
> +raise error.Abort(_("cannot change branch in betwen the stack"))
> +

typo: betwen

Also, I'm not sure "between" is appropriate here. Maybe "cannot change branch 
in the middle of a stack"?

> cmdutil.py:770
> +
> +ui.debug("changing branch of '%s' from '%s' to '%s'" % (
> +hex(ctx.node()), oldbranch, label))

nit: with `%` on the continuation line and alignment with opening parenthesis, 
it'd be more readable.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1074

To: pulkit, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1082: split: new extension to split changesets

[dlax (Denis Laxalde)]

dlax added inline comments.

INLINE COMMENTS

> split.py:53
> +
> +Repetitively prompt changes and commit message for new changesets until
> +there is nothing left in the original changeset.

Not sure "repetitively" exists. Maybe "repeatedly"?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1082

To: quark, #hg-reviewers
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1073: commands: move a bunch of statements into if True for next patch

[dlax (Denis Laxalde)]

dlax requested changes to this revision.
dlax added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> commands.py:1048
>  scmutil.checknewlabel(repo, label, 'branch')
> -repo.dirstate.setbranch(label)
> -ui.status(_('marked working directory as branch %s\n') % label)
> -
> -# find any open named branches aside from default
> -others = [n for n, h, t, c in repo.branchmap().iterbranches()
> -  if n != "default" and not c]
> -if not others:
> -ui.status(_('(branches are permanent and global, '
> -'did you want a bookmark?)\n'))
> +if True:
> +repo.dirstate.setbranch(label)

Instead of this, you could use an early `return` in the next patch as:

  --- a/mercurial/commands.py
  +++ b/mercurial/commands.py
  @@ -1045,6 +1045,10 @@ def branch(ui, repo, label=None, **opts)
# i18n: "it" refers to an existing 
branch
hint=_("use 'hg update' to switch to 
it"))
   scmutil.checknewlabel(repo, label, 'branch')
  +
  +if revs:
  +return cmdutil.changebranch(ui, repo, revs, label)
  +
   repo.dirstate.setbranch(label)
   ui.status(_('marked working directory as branch %s\n') % label)

and drop this one.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1073

To: pulkit, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1072: tersestatus: rework dirnode and tersedir docstrings

[dlax (Denis Laxalde)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG23eb03f46929: tersestatus: rework dirnode and tersedir 
docstrings (authored by dlax, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1072?vs=2730=2748

REVISION DETAIL
  https://phab.mercurial-scm.org/D1072

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -409,9 +409,8 @@
 
 class dirnode(object):
 """
-represents a directory in user working copy
-
-stores information which is required for purpose of tersing the status
+Represent a directory in user working copy with information required for
+the purpose of tersing its status.
 
 path is the path to the directory
 
@@ -431,16 +430,16 @@
 self.subdirs = {}
 
 def _addfileindir(self, filename, status):
-""" adds a file in this directory as the direct child """
+"""Add a file in this directory as a direct child."""
 self.files.append((filename, status))
 
 def addfile(self, filename, status):
 """
-adds a file which is present in this directory to its direct parent
-dirnode object
-
-if the file is not direct child of this directory, we traverse to the
-directory of which this file is a direct child of and add the file 
there
+Add a file to this directory or to its direct parent directory.
+
+If the file is not direct child of this directory, we traverse to the
+directory of which this file is a direct child of and add the file
+there.
 """
 
 # the filename contains a path separator, it means it's not the direct
@@ -463,27 +462,14 @@
 self.statuses.add(status)
 
 def iterfilepaths(self):
-"""
-adds files to the their respective status list in the final tersed list
-
-path is the path of parent directory of the file
-files is a list of tuple where each tuple is (filename, status)
-"""
+"""Yield (status, path) for files directly under this directory."""
 for f, st in self.files:
 yield st, os.path.join(self.path, f)
 
 def tersewalk(self, terseargs):
 """
-a recursive function which process status for a certain directory.
-
-self is an oject of dirnode class defined below. each object of dirnode
-class has a set of statuses which files in that directory has. This 
ease
-our check whether we can terse that directory or not.
-
-tersedict is a dictonary which contains each status abbreviation as key
-and list of files and tersed dirs in that status as value. In each
-function call we are passing the same dict and adding files and dirs
-to it.
+Yield (status, path) obtained by processing the status of this
+dirnode.
 
 terseargs is the string of arguments passed by the user with `--terse`
 flag.
@@ -494,7 +480,7 @@
 subdirectories) share the same status and the user has asked us to 
terse
 that status. -> yield (status, dirpath)
 
-2) If '1)' does not happen, we do following:
+2) Otherwise, we do following:
 
 a) Yield (status, filepath)  for all the files which are in 
this
 directory (only the ones in this directory, not the 
subdirs)
@@ -523,7 +509,7 @@
 
 def tersedir(statuslist, terseargs):
 """
-terses the status if all the files in a directory shares the same status
+Terse the status if all the files in a directory shares the same status.
 
 statuslist is scmutil.status() object which contains a list of files for
 each status.
@@ -533,10 +519,6 @@
 The function makes a tree of objects of dirnode class, and at each node it
 stores the information required to know whether we can terse a certain
 directory or not.
-
-tersedict (defined in the function) is a dictionary which has one word key
-for each status and a list of files and dir in that status as the 
respective
-value.
 """
 # the order matters here as that is used to produce final list
 allst = ('m', 'a', 'r', 'd', 'u', 'i', 'c')



To: dlax, #hg-reviewers, pulkit, durin42
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1072: tersestatus: rework dirnode and tersedir docstrings

[dlax (Denis Laxalde)]

dlax created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Follow-up on refactorings 
https://phab.mercurial-scm.org/rHG3d6d4b12128e8056f988bf36be1fc46e4c7b29dc and 
https://phab.mercurial-scm.org/rHG5d98674df18ab8aeaa4a27890061b1e1b99b6f3b of 
the original
  changeset 
https://phab.mercurial-scm.org/rHG7e3001b74ab398ef4fc6cad25cc32148f2104dc5 by 
updating the docstrings of dirnode class and tersedir
  function:
  
  - rewrite dirnode.iterfilepaths()'s docstring (the method got renamed and 
reimplemented in 
https://phab.mercurial-scm.org/rHG5d98674df18ab8aeaa4a27890061b1e1b99b6f3b);
  - simplify and update dirnode.tersewalk() to remove reference to 'self' and 
'tersedict';
  - use the imperative form of verbs in the first sentence of all docstrings.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1072

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -409,9 +409,8 @@
 
 class dirnode(object):
 """
-represents a directory in user working copy
-
-stores information which is required for purpose of tersing the status
+Represent a directory in user working copy with information required for
+the purpose of tersing its status.
 
 path is the path to the directory
 
@@ -431,16 +430,16 @@
 self.subdirs = {}
 
 def _addfileindir(self, filename, status):
-""" adds a file in this directory as the direct child """
+"""Add a file in this directory as a direct child."""
 self.files.append((filename, status))
 
 def addfile(self, filename, status):
 """
-adds a file which is present in this directory to its direct parent
-dirnode object
-
-if the file is not direct child of this directory, we traverse to the
-directory of which this file is a direct child of and add the file 
there
+Add a file to this directory or to its direct parent directory.
+
+If the file is not direct child of this directory, we traverse to the
+directory of which this file is a direct child of and add the file
+there.
 """
 
 # the filename contains a path separator, it means it's not the direct
@@ -463,27 +462,14 @@
 self.statuses.add(status)
 
 def iterfilepaths(self):
-"""
-adds files to the their respective status list in the final tersed list
-
-path is the path of parent directory of the file
-files is a list of tuple where each tuple is (filename, status)
-"""
+"""Yield (status, path) for files directly under this directory."""
 for f, st in self.files:
 yield st, os.path.join(self.path, f)
 
 def tersewalk(self, terseargs):
 """
-a recursive function which process status for a certain directory.
-
-self is an oject of dirnode class defined below. each object of dirnode
-class has a set of statuses which files in that directory has. This 
ease
-our check whether we can terse that directory or not.
-
-tersedict is a dictonary which contains each status abbreviation as key
-and list of files and tersed dirs in that status as value. In each
-function call we are passing the same dict and adding files and dirs
-to it.
+Yield (status, path) obtained by processing the status of this
+dirnode.
 
 terseargs is the string of arguments passed by the user with `--terse`
 flag.
@@ -494,7 +480,7 @@
 subdirectories) share the same status and the user has asked us to 
terse
 that status. -> yield (status, dirpath)
 
-2) If '1)' does not happen, we do following:
+2) Otherwise, we do following:
 
 a) Yield (status, filepath)  for all the files which are in 
this
 directory (only the ones in this directory, not the 
subdirs)
@@ -523,7 +509,7 @@
 
 def tersedir(statuslist, terseargs):
 """
-terses the status if all the files in a directory shares the same status
+Terse the status if all the files in a directory shares the same status.
 
 statuslist is scmutil.status() object which contains a list of files for
 each status.
@@ -533,10 +519,6 @@
 The function makes a tree of objects of dirnode class, and at each node it
 stores the information required to know whether we can terse a certain
 directory or not.
-
-tersedict (defined in the function) is a dictionary which has one word key
-for each status and a list of files and dir in that status as the 
respective
-value.
 """
 # the order matters here as that is used to produce final list
 allst = ('m', 'a', 'r', 'd', 'u', 'i', 'c')



To: dlax, #hg-reviewers
Cc: mercurial-devel
___

D1042: tersestatus: make methods part of the dirnode class

[dlax (Denis Laxalde)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG3d6d4b12128e: tersestatus: make methods part of the dirnode 
class (authored by dlax, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1042?vs=2646=2717

REVISION DETAIL
  https://phab.mercurial-scm.org/D1042

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -462,59 +462,64 @@
 if status not in self.statuses:
 self.statuses.add(status)
 
-def _addfilestotersed(path, files, tersedict):
-""" adds files to the their respective status list in the final tersed list
-
-path is the path of parent directory of the file
-files is a list of tuple where each tuple is (filename, status)
-tersedict is a dictonary which contains each status abbreviation as key and
-list of files and tersed dirs in that status as value
-"""
-for f, st in files:
-tersedict[st].append(os.path.join(path, f))
-
-def _processtersestatus(subdir, tersedict, terseargs):
-"""a recursive function which process status for a certain directory.
-
-subdir is an oject of dirnode class defined below. each object of dirnode
-class has a set of statuses which files in that directory has. This ease 
our
-check whether we can terse that directory or not.
-
-tersedict is a dictonary which contains each status abbreviation as key and
-list of files and tersed dirs in that status as value. In each function 
call
-we are passing the same dict and adding files and dirs to it.
-
-terseargs is the string of arguments passed by the user with `--terse` 
flag.
-
-Following are the cases which can happen:
-
-1) All the files in the directory (including all the files in its
-subdirectories) share the same status and the user has asked us to terse
-that status. -> we add the directory name to status list and return
-
-2) If '1)' does not happen, we do following:
-
-a) Add all the files which are in this directory (only the ones in
-this directory, not the subdirs) to their respective status 
list
-
-b) Recurse the function on all the subdirectories of this directory
-"""
-
-if len(subdir.statuses) == 1:
-onlyst = subdir.statuses.pop()
-
-# Making sure we terse only when the status abbreviation is passed as
-# terse argument
-if onlyst in terseargs:
-tersedict[onlyst].append(subdir.path + pycompat.ossep)
-return
-
-# add the files to status list
-_addfilestotersed(subdir.path, subdir.files, tersedict)
-
-#recurse on the subdirs
-for dirobj in subdir.subdirs.values():
-_processtersestatus(dirobj, tersedict, terseargs)
+def _addfilestotersed(self, tersedict):
+"""
+adds files to the their respective status list in the final tersed list
+
+path is the path of parent directory of the file
+files is a list of tuple where each tuple is (filename, status)
+tersedict is a dictonary which contains each status abbreviation as 
key and
+list of files and tersed dirs in that status as value
+"""
+for f, st in self.files:
+tersedict[st].append(os.path.join(self.path, f))
+
+def _processtersestatus(self, tersedict, terseargs):
+"""
+a recursive function which process status for a certain directory.
+
+self is an oject of dirnode class defined below. each object of dirnode
+class has a set of statuses which files in that directory has. This 
ease
+our check whether we can terse that directory or not.
+
+tersedict is a dictonary which contains each status abbreviation as key
+and list of files and tersed dirs in that status as value. In each
+function call we are passing the same dict and adding files and dirs
+to it.
+
+terseargs is the string of arguments passed by the user with `--terse`
+flag.
+
+Following are the cases which can happen:
+
+1) All the files in the directory (including all the files in its
+subdirectories) share the same status and the user has asked us to 
terse
+that status. -> we add the directory name to status list and return
+
+2) If '1)' does not happen, we do following:
+
+a) Add all the files which are in this directory (only the 
ones in
+this directory, not the subdirs) to their respective 
status list
+
+b) Recurse the function on all the subdirectories of this
+   directory
+"""
+
+if len(self.statuses) == 1:
+onlyst = self.statuses.pop()
+
+# Making sure we terse only when the status abbreviation is
+# passed as terse argument
+   

D1043: tersestatus: avoid modifying tersedict

[dlax (Denis Laxalde)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG5d98674df18a: tersestatus: avoid modifying tersedict 
(authored by dlax, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1043?vs=2647=2718

REVISION DETAIL
  https://phab.mercurial-scm.org/D1043

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -462,19 +462,17 @@
 if status not in self.statuses:
 self.statuses.add(status)
 
-def _addfilestotersed(self, tersedict):
+def iterfilepaths(self):
 """
 adds files to the their respective status list in the final tersed list
 
 path is the path of parent directory of the file
 files is a list of tuple where each tuple is (filename, status)
-tersedict is a dictonary which contains each status abbreviation as 
key and
-list of files and tersed dirs in that status as value
 """
 for f, st in self.files:
-tersedict[st].append(os.path.join(self.path, f))
-
-def _processtersestatus(self, tersedict, terseargs):
+yield st, os.path.join(self.path, f)
+
+def tersewalk(self, terseargs):
 """
 a recursive function which process status for a certain directory.
 
@@ -494,12 +492,12 @@
 
 1) All the files in the directory (including all the files in its
 subdirectories) share the same status and the user has asked us to 
terse
-that status. -> we add the directory name to status list and return
+that status. -> yield (status, dirpath)
 
 2) If '1)' does not happen, we do following:
 
-a) Add all the files which are in this directory (only the 
ones in
-this directory, not the subdirs) to their respective 
status list
+a) Yield (status, filepath)  for all the files which are in 
this
+directory (only the ones in this directory, not the 
subdirs)
 
 b) Recurse the function on all the subdirectories of this
directory
@@ -511,15 +509,17 @@
 # Making sure we terse only when the status abbreviation is
 # passed as terse argument
 if onlyst in terseargs:
-tersedict[onlyst].append(self.path + pycompat.ossep)
+yield onlyst, self.path + pycompat.ossep
 return
 
 # add the files to status list
-self._addfilestotersed(tersedict)
+for st, fpath in self.iterfilepaths():
+yield st, fpath
 
 #recurse on the subdirs
 for dirobj in self.subdirs.values():
-dirobj._processtersestatus(tersedict, terseargs)
+for st, fpath in dirobj.tersewalk(terseargs):
+yield st, fpath
 
 def tersedir(statuslist, terseargs):
 """
@@ -536,7 +536,7 @@
 
 tersedict (defined in the function) is a dictionary which has one word key
 for each status and a list of files and dir in that status as the 
respective
-value. The dictionary is passed to other helper functions which builds it.
+value.
 """
 # the order matters here as that is used to produce final list
 allst = ('m', 'a', 'r', 'd', 'u', 'i', 'c')
@@ -558,11 +558,13 @@
 tersedict[attrname[0]] = []
 
 # we won't be tersing the root dir, so add files in it
-rootobj._addfilestotersed(tersedict)
+for st, fpath in rootobj.iterfilepaths():
+tersedict[st].append(fpath)
 
 # process each sub-directory and build tersedict
 for subdir in rootobj.subdirs.values():
-subdir._processtersestatus(tersedict, terseargs)
+for st, f in subdir.tersewalk(terseargs):
+tersedict[st].append(f)
 
 tersedlist = []
 for st in allst:



To: pulkit, #hg-reviewers, durin42
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel