This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2400

stack: begin to make the stack revset configurable
AbandonedPublic

Authored by lothiraldan on Feb 23 2018, 5:37 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Summary

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

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

lothiraldan created this revision.Feb 23 2018, 5:37 AM
lothiraldan updated this revision to Diff 6216.Feb 28 2018, 6:59 PM
indygreg accepted this revision.Mar 19 2018, 4:08 PM
indygreg added a subscriber: indygreg.

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.

mercurial/stack.py
41–42

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.

This revision is now accepted and ready to land.Mar 19 2018, 4:08 PM
indygreg requested changes to this revision.Mar 19 2018, 4:14 PM

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?

This revision now requires changes to proceed.Mar 19 2018, 4:14 PM
lothiraldan added a comment.Apr 5 2018, 11:43 AM

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.

lothiraldan updated this revision to Diff 7733.Apr 5 2018, 12:22 PM
lothiraldan abandoned this revision.Oct 5 2019, 12:03 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/configitems.py (3 lines)
Mmercurial/stack.py (22 lines)

Diff 7733

mercurial/configitems.py

default='none', default='none',
) )
coreconfigitem('smtp', 'username', coreconfigitem('smtp', 'username',
default=None, default=None,
) )
coreconfigitem('sparse', 'missingwarning', coreconfigitem('sparse', 'missingwarning',
default=True, default=True,
) )
coreconfigitem('stack', 'not-merge',
default=True,
)
coreconfigitem('subrepos', 'allowed', coreconfigitem('subrepos', 'allowed',
default=dynamicdefault, # to make backporting simpler default=dynamicdefault, # to make backporting simpler
) )
coreconfigitem('subrepos', 'hg:allowed', coreconfigitem('subrepos', 'hg:allowed',
default=dynamicdefault, default=dynamicdefault,
) )
coreconfigitem('subrepos', 'git:allowed', coreconfigitem('subrepos', 'git:allowed',
default=dynamicdefault, default=dynamicdefault,

mercurial/stack.py

# stack.py - Mercurial functions for stack definition # stack.py - Mercurial functions for stack definition
# #
# Copyright Matt Mackall <mpm@selenic.com> and other # Copyright Matt Mackall <mpm@selenic.com> and other
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
from . import ( from . import (
revsetlang, revsetlang,
scmutil, scmutil,
) )
baserevspec = "only(%s) and not public()"
def getstack(repo, rev=None): def getstack(repo, rev=None):
"""return a sorted smartrev of the stack containing either rev if it is """return a sorted smartrev of the stack containing either rev if it is
not None or the current working directory parent. not None or the current working directory parent.
The stack will always contain all drafts changesets which are ancestors to 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: if rev is None:
rev = '.' rev = '.'
revspec = 'reverse(only(%s) and not public() and not ::merge())' revspecargs = [revsetlang.formatspec(baserevspec, rev)]
revset = revsetlang.formatspec(revspec, 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)
indygregUnsubmitted
Not Done

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.

indygreg: As I review just this patch, I question if we shouldn't just concatenate `and not ::merge()` to…
revisions = scmutil.revrange(repo, [revset]) revisions = scmutil.revrange(repo, [revset])
revisions.sort() revisions.sort()
return revisions return revisions