RFC: Phase UI (revset, phase command and others)

[Gilles Moris]

On Wednesday 28 December 2011 12:23:23 am Pierre-Yves David wrote:
> On 27 déc. 2011, at 23:22, Gilles Moris wrote:
> > On Tuesday 27 December 2011 12:11:12 am Pierre-Yves David wrote:
> >> Phase Appearance In UI
> >> ======================
> >>
> >> Data related to phase will appear in several places:
> >>
> >> Changesets Log
> >> -----------------
> >>
> >> hg log output will include phase name for changeset not in the public
> >> phase.
> >>
> >>   changeset:   15781:2ebe3d0ce91d
> >>   branch:      stable
> >>   user:        FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> >>   date:        Fri Dec 16 21:09:41 2011 +0900
> >>   summary:     i18n: use encoding.lower/upper for encoding aware case
> >> folding phase:       draft
> >>
> >> This also applies to other commands with similar output
> >>
> >> On the template side, {phase} and {phasestr}  keyword can be used to
> >> display phase number and phase name of a changeset.
> >
> > I don't think we should expose the numerical phase number.
> > I feel this is more a detail of implementation that shall be hidden from
> > the user.
>
> It's implementation details but it highlight the fact than phase are
> ordered and may help user to understand what is going on with phase.
>
> Do you suggest to have {phase} and {phasenum} keywords or only a string
> related one ?
>

I was thinking to just implement {phase} as the string one. Then {phasenum} 
can still be implemented at a later time if really needed.

> > Moreover, I would renumber phases from 0,1,2 to 10,20,30 or similar to
> > allow some spare numbers if adding intermediate phase is needed (as
> > suggested below in solution A).
>
> I find this spare number a quite interesting topic. But I did not processed
> enough high priority item about phase to care about it yet. If you want to
> drive a discussion on the topic of "what more phase could be useful for and
> what are the draw back", go for it ! I'll try to feed it.
>

No, I do not have any additional phase in mind. Just the idea that we may be 
into trouble if we find an interresting intermediate phase concept after 2.1.

> > If you don't want to renumber, making those numbers private could allow a
> > future renumbering under the hood.
>
> Renumbering under the hood will probably be hard anyway so it need to be
> decided before 2.1.
>

Yes, this is the main reason why I've raised my hand now again.

> >> Phase Movement
> >> --------------
> >>
> >> The user will be informed of phase movement the same way changegroup
> >> addition are notified (check last line):
> >>
> >>   % 22:31 marmoute at yamac ~/src/pylint > hg pushing
> >>   real URL is http://hg.logilab.org/pylint
> >>   pushing to http://hg.logilab.org/pylint
> >>   searching for changes
> >>   adding changesets
> >>   adding manifests
> >>   adding file changes
> >>   added 60 changesets with 173 changes to 314 files
> >>   90 changesets changed phases
> >>
> >> more verbose output may provide additional information:
> >>
> >>   90 changesets changed phases
> >>   73 changesets moved to public phase
> >>   17 changesets moved to draft phase
> >>
> >> This applies to any command changing the phase of changesets.
> >>
> >> Extension
> >> -----------
> >>
> >> Extensions rewriting history will refuse to work on immutable changeset:
> >>
> >>   abort: Can't rebase immutable changeset e1c4361dd923
> >>   (see hg help phases for details)
> >>
> >> Impacted core extensions are: mq and rebase
> >>
> >> revset
> >> -------
> >>
> >> revset to select changeset according to their phase will be implemented.
> >>
> >> The naive solution is:
> >>
> >>   public()     match changeset in public phase.
> >>   draft()      match changeset in draft phase.
> >>   secret()     match changeset in secret phase.
> >>
> >> But in practice we want to be able to do more complicated queries:
> >>
> >>   * "Match all changeset at least draft" --> (draft() + secret())
> >>   * "Match all changeset at most draft" --> (draft() + public())
> >>
> >> There are multiple options to achieve this.
> >>
> >> Solution A: add more revset symbols
> >> ```````````````````````````````````````````````````
> >>
> >> Adding "exchanged()" and "mutable()" should do it.
> >>
> >> pro:
> >>  * simple to use
> >> con:
> >>  * multiplies symbols
> >>  * Does not scale well if we add phases
> >>
> >> Solution B: add argument to revset
> >> ``````````````````````````````````````````````````
> >>
> >> We add an "operator" argument to revset to control how it should be
> >> interpreted:
> >>
> >>   Match any changeset in phase draft or lower --> "draft('<=')"
> >>
> >> Possible values are '<', '<=', '='(default), '>=', '>'.
> >>
> >> pro:
> >>   * keep simple usage simple
> >>   * cover most things people will want to express (except phase between
> >> X and Y) cons:
> >>   * complicated usecase are a bit more complicated
> >>
> >>
> >> Solution C: Use a single symbol
> >> ``````````````````````````````````````````````````
> >>
> >> We can use a single "phases" revset taking a "phase-set" in argument:
> >>
> >>   match changesets in draft phase --> phase('draft)
> >>   match changesets in draft phase or lower --> phase('<=draft)
> >>   match changeset between public and secret phase -->
> >> phase('public::draft')
> >>
> >> pro:
> >>   * Add less symbol
> >>   * everything is done using the same symbol
> >>   * complex case are simpler to express
> >>   * cover any complicated case people might want
> >> cons:
> >>   * Make simple case more verbose
> >
> > Slightly prefer solution C as it is explicit.
> > In particular "phase('public::draft')" because it is consistent with
> > revsets. Anyway, revsets already have some aliases like author/user, so a
> > mix of solution A and C shall be possible.
>
> Benoit Boissinot expressed such option in private too. I quite like it.
>
> >> Phase command
> >> =============
> >>
> >> Phase of changeset can be manually altered with the "hg phases" command
> >>
> >> hg phases [-C|-p|-d|-s] [-b|-e] [revset]
> >>
> >> set or show phase of changeset
> >>
> >> If no revset is provided, it applies to the current working directory
> >> phase.
> >>
> >> options:
> >>
> >> -p --public   Set target into public phase
> >> -d --draft    Set target into draft phase (or leave it in lower phase
> >> (public)) -s --secret   Set target into secret phase (or leave it in
> >> lower phase (draft and public))
> >>
> >>
> >> -b --backward reverse the logic, phase are left in upper phase, not
> >> lower -e --exact    all target changeset are exactly set in the
> >> specified phase
> >>
> >> -C --clear    reset next commit phase to default (max of parent phase
> >> and phases.new-commit option)
> >>
> >>
> >> Exact help text is to be tuned to remove the confusing lower//upper
> >> phase stuff.
> >
> > I am quite confused with the command, in particular because the essential
> > rule "A child changeset can not be in a lower phase than its parents" is
> > not expressed in the help. Changing phase in an ancestor can change phase
> > in the descendants.
>
> The command respect the "A child changeset can not be in a lower phase than
> its parents" rules. It's ommited from the help because there is basically
> not help in my proposal. I note this suggestion for the real help.
>
> To deconfuse you,
>
> Moving boundary forward to a changeset can change it's ancestor phase.
> Moving boundary backward to a changeset can change it's descendant phase.
>
> > So I think that "--exact" may probably be the default and only behavior
> > for the time being, as the lower/upper/backward makes it more (too much?)
> > complex.
>
> I think that by default the command should not move boundary backward.
>
> The --backward switch is here to allow people to move the boundary backward
> anyway (whil not worryhing about moving it forward by mistake)
>
> The --exact switch is here to allow both --backward and --forward[1] to be
> specified in a single command. Internaly both backward and forward move
> will be used.
>
> [1] --forward don't exist in my proposal

I like the suggestions in other threads:
- use promote/demote instead of forward/backward in the description: easier to 
understand
- by default promote/move forward, and use --force to demote: less options, so 
easier to jump in
- not sure -C is needed

Regards.
Gilles.