[PATCH evolve_ext V2] fold: take an explicit list of revisions (BC)

Greg Ward greg at gerg.ca
Thu Jun 26 07:41:14 CDT 2014


On 24 June 2014, Jordi Gutiérrez Hermoso said:
> On Sun, 2014-06-22 at 19:52 -0400, Greg Ward wrote:
> > # HG changeset patch
> > # User Greg Ward <greg at gerg.ca>
> > # Date 1403481137 14400
> > #      Sun Jun 22 19:52:17 2014 -0400
> > # Node ID 4ab7a80fc11f275c03d4ddb94936a0688b71e6bc
> > # Parent  2fbba0bf7e7c8cbff1f94bc95c4d6214df85ef81
> > fold: take an explicit list of revisions (BC)
> 
> I've got the following WIP instead. Still missing tests, which is why
> I haven't patchbombed it yet.
> 
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh at octave.org>
> # Date 1397267702 14400
> #      Fri Apr 11 21:55:02 2014 -0400
> # Branch stable
> # Node ID 2e7527f1da736908db6100da5fb24108d43cc3ea
> # Parent  ff43167ed0ba60038304a08621bda4cbac5e6224
> fold: rehaul handling of multiple and single revisions (BC)
        ^^^^^^

"overhaul"

> The fold command parsed the revision arguments in a very peculiar and
> idiosyncratic fashion: either a single revision could be specified or
> multiple revisions could be specified with --rev (but not both). This
> is inconsistent with the way all other hg commands parse revision
> arguments. We have several examples of command where several revisions
> are passed, and the --rev option is optional for specifying those
> revisions (update, strip, export).
> 
> This patch alters the way in which fold parses its revision arguments.
> No distinction is made between revisions passed with or without the
> --rev argument. Whether a single or multiple revision is specified, it
> will all be folded together into one, by connecting them to the parent
> of the working directory. If the --exact argument is passed, then the
> parent of the working directory is ignored and the specified revisions
> must be a single contiguous line.

*IF* we accept Pierre-Yves' judgement that "specified revision and all
of its descendants up to ." should be the default, then there is no
need to accept multiple revisions. Just make it

  hg fold [-r] REV

and only accept one REV.

> diff --git a/hgext/evolve.py b/hgext/evolve.py
> --- a/hgext/evolve.py
> +++ b/hgext/evolve.py
> @@ -2053,41 +2053,58 @@ def touch(ui, repo, *revs, **opts):
>          lockmod.release(lock, wlock)
>  
>  @command('^fold|squash',
> -    [('r', 'rev', [], _("explicitly specify the full set of revision to fold")),
> +    [('r', 'rev', [], _("revision to fold")),
> +     ('', 'exact', None, _("ignore parent of working directory"))
>      ] + commitopts + commitopts2,
> -    # allow to choose the seed ?
> -    _('rev'))
> +    _('hg fold [OPTION]... [-r] REV'))

Right! Like that usage message!

>  def fold(ui, repo, *revs, **opts):
> -    """Fold multiple revisions into a single one
> +    """fold multiple revisions into a single one
>  
> -    The revisions from your current working directory to the given one are folded
> -    into a single successor revision.
> +    Folds all revisions between the specified revision and the parent
> +    of working directory into a single revision. The folded revisions
> +    will be marked as obsolete and replaced by the resulting revision.

Yes.

> -    you can alternatively use --rev to explicitly specify revisions to be folded,
> -    ignoring the current working directory parent.
> +    If multiple revisions are specified, they will all be folded
> +    together with the parent of the working directory, along with any
> +    intermediate revisions.
> +
> +    If specifying multiple revisions, use --exact for folding those
> +    revisions while ignoring the parent of the working directory. In
> +    this case, the given revisions must form a linear unbroken chain.

No. Yuck. Confusing. Do not like.

>      """
>      revs = list(revs)
> -    if revs:
> -        if opts.get('rev', ()):
> -            raise util.Abort("cannot specify both --rev and a target revision")
> -        targets = scmutil.revrange(repo, revs)
> -        revs = repo.revs('(%ld::.) or (.::%ld)', targets, targets)
> -    elif 'rev' in opts:
> -        revs = scmutil.revrange(repo, opts['rev'])
> -    else:
> -        revs = ()
> +    revs.extend(opts['rev'])

Assuming this is the best way to implement "-r before REV is
optional", all you need to add is

  if len(revs) > 1:
      raise util.Abort(_('too many revisions specified'))
  rev = revs[0]

and then forget about handling multiple revs.

>      if not revs:
> -        ui.write_err('no revision to fold\n')
> +        ui.write_err(_('no revisions specified\n'))
>          return 1

FWIW, graft just aborts in this case. (But see my comment below about
mixing multiple changes together: you're doing it here.)

> +    if len(revs) == 1:
> +        ui.write_err(_('single revision specified, nothing to fold\n'))
> +        return 2

Oh yeah! I forgot to do this in my patch. Good catch.

> +
>      roots = repo.revs('roots(%ld)', revs)
>      if len(roots) > 1:
> -        raise util.Abort("set has multiple roots")
> +        raise util.Abort(_("cannot fold non-linear revisions"),
> +                           hint=_("multiple roots detected"))

You're mixing multiple changes together. Improving error messages,
adding hints, and I18N are all good things ... but they are distinct
things from "overhaul handling of revisions".

       Greg
-- 
Greg Ward                            http://www.gerg.ca
<greg at gerg.ca>                       @gergdotca


More information about the Mercurial-devel mailing list