Patchwork D2400: stack: begin to make the stack revset configurable

login
register
mail settings
Submitter phabricator
Date Feb. 23, 2018, 10:37 a.m.
Message ID <differential-rev-PHID-DREV-yojh5k7yhho4nznho2p5-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/28272/
State New
Headers show

Comments

phabricator - Feb. 23, 2018, 10:37 a.m.
lothiraldan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  First extrat the not-merge part in its own configuration knob `stack.not-
  merge`.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/configitems.py
  mercurial/stack.py

CHANGE DETAILS




To: lothiraldan, #hg-reviewers
Cc: mercurial-devel
phabricator - March 19, 2018, 8:08 p.m.
indygreg accepted this revision.
indygreg added a comment.
This revision is now accepted and ready to land.


  I'm not super keen on introducing a new `[stack]` config section. But I don't have any better ideas for where this should go. `[ui]` is my best idea, but I thought we decided we wanted to stop stuffing random stuff into `[ui]`. So I dunno.

INLINE COMMENTS

> stack.py:41-42
> +
> +    finalrevspec = " and ".join(revspec)
> +    revset = revsetlang.formatspec(finalrevspec, *revspecargs)
>      revisions = scmutil.revrange(repo, [revset])

As I review just this patch, I question if we shouldn't just concatenate `and not ::merge()` to the revset string. But I know other changes are coming. I assume the final state justifies the extra complexity.

REPOSITORY
  rHG Mercurial

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

To: lothiraldan, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel
phabricator - March 19, 2018, 8:14 p.m.
indygreg requested changes to this revision.
indygreg added a comment.
This revision now requires changes to proceed.


  Reading through the remainder of the series, I like the customization of stack evaluation. But the use of config options seems weird to me. It isn't clear why we need config options. It feels like these options should be passed in as function arguments. Now, it will be useful to test these variations. Maybe that does require config options. But until there is a user-facing need for these config options, It feels better to put them in `[devel]` or something like that.
  
  Is there a future consumer of these config options that justifies them being normal config options?

REPOSITORY
  rHG Mercurial

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

To: lothiraldan, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel
phabricator - April 5, 2018, 3:43 p.m.
lothiraldan added a comment.


  The goal of this series is to define a clean API so that commands and tools can be built in a stack-definition agnostic way. It's not to define a one-size-fits-all stack definition.
  
  People and companies have different workflows and needs. Their definition of a stack will likely to be different to better match their workflow. I tried finding a way to configure stack as easily as possible. I have the impression that using configuration options is slightly easier than defining a custom extension. I also avoided using directly a revset from the configuration file as I'm not sure it would be the best way to express all situations.
  
  I don't know if the config options best place are in the `stack` section though.

REPOSITORY
  rHG Mercurial

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

To: lothiraldan, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel

Patch

diff --git a/mercurial/stack.py b/mercurial/stack.py
--- a/mercurial/stack.py
+++ b/mercurial/stack.py
@@ -9,18 +9,34 @@ 
 
 from . import revsetlang, scmutil
 
+baserevspec = "only(%s) and not public()"
+
 def getstack(repo, rev=None):
     """return a sorted smartrev of the stack containing either rev if it is
     not None or the current working directory parent.
 
     The stack will always contain all drafts changesets which are ancestors to
-    the revision and are not merges.
+    the revision.
+
+    There are several config options to restrict the changesets that will be
+    part of the stack:
+
+    [stack]
+    not-merge = (boolean) # The stack will contains only non-merge changesets
+                          # if set to True (default: True)
     """
     if rev is None:
         rev = '.'
 
-    revspec = 'reverse(only(%s) and not public() and not ::merge())'
-    revset = revsetlang.formatspec(revspec, rev)
+    revspecargs = [revsetlang.formatspec(baserevspec, rev)]
+    revspec = ["%r"]
+
+    if repo.ui.configbool("stack", "not-merge"):
+        revspecargs.append("not ::merge()")
+        revspec.append("%r")
+
+    finalrevspec = " and ".join(revspec)
+    revset = revsetlang.formatspec(finalrevspec, *revspecargs)
     revisions = scmutil.revrange(repo, [revset])
     revisions.sort()
     return revisions
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -950,6 +950,9 @@ 
 coreconfigitem('sparse', 'missingwarning',
     default=True,
 )
+coreconfigitem('stack', 'not-merge',
+    default=True,
+)
 coreconfigitem('subrepos', 'allowed',
     default=dynamicdefault,  # to make backporting simpler
 )