[PATCH v2] histedit: use one editor when multiple folds happen in a row (issue3524) (BC)

Augie Fackler raf at durin42.com
Tue Sep 1 14:59:09 CDT 2015


On Tue, Sep 1, 2015 at 1:23 PM, Durham Goode <durham at fb.com> wrote:
> I agree, it's a little beyond what we need, but given histedit's history, I
> think it'd be worth doing.
>
> It might also enable other interesting things from extensions.  For
> instance, if we wanted to remove the need to specify 'drop' lines, we could
> have the drop action class preprocess the list to add 'drop' entries when
> the user has deleted lines.  Or if we wanted to add a 'review' action, the
> action could be implemented as a preprocessor that replaces 'review' actions
> with 'exec run_my_review_tool'

So, I've started working on this, and I'm of the opinion that it's
something we should do later. Reason being, it's underspecified:

How do I know what order to run the preprocessors in? How do I know
that my preprocessor is going to run late enough in the stack to do
what I want? Or early enough?

I think we should wait until there's a concrete second user to
actually do this refactor. Note that I'm *fine* with that second user
being out of tree, I just can't quite figure out what the Right API is
going to be today because my crystal ball is still broken.

>
>
> On 9/1/15 10:17 AM, Augie Fackler wrote:
>>
>> Yeah, I can do that. Sounds a little overengineered for the current
>> case, but I can see it being useful. I'll roll a v3 soon. Thanks!
>>
>> On Tue, Sep 1, 2015 at 1:13 PM, Durham Goode <durham at fb.com> wrote:
>>>
>>>
>>> On 8/31/15 9:56 PM, Augie Fackler wrote:
>>>>
>>>> # HG changeset patch
>>>> # User Augie Fackler <augie at google.com>
>>>> # Date 1440701186 14400
>>>> #      Thu Aug 27 14:46:26 2015 -0400
>>>> # Node ID 6d4eb98524de846543455102dfe4aaa3c8c8af45
>>>> # Parent  049005de325ea400893f45bd6221215cc9b26db0
>>>> histedit: use one editor when multiple folds happen in a row (issue3524)
>>>> (BC)
>>>>
>>>> This was the first ever feature request for histedit, originally filed
>>>> back on April 4, 2009. Finally fixed.
>>>>
>>>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>>>> --- a/hgext/histedit.py
>>>> +++ b/hgext/histedit.py
>>>> @@ -559,6 +559,9 @@ class fold(histeditaction):
>>>>        def skipprompt(self):
>>>>            return False
>>>>    +    def mergedescs(self):
>>>> +        return True
>>>> +
>>>>        def finishfold(self, ui, repo, ctx, oldctx, newnode,
>>>> internalchanges):
>>>>            parent = ctx.parents()[0].node()
>>>>            hg.update(repo, parent)
>>>> @@ -566,7 +569,7 @@ class fold(histeditaction):
>>>>            commitopts = {}
>>>>            commitopts['user'] = ctx.user()
>>>>            # commit message
>>>> -        if self.skipprompt():
>>>> +        if not self.mergedescs():
>>>>                newmessage = ctx.description()
>>>>            else:
>>>>                newmessage = '\n***\n'.join(
>>>> @@ -601,7 +604,22 @@ class fold(histeditaction):
>>>>                replacements.append((ich, (n,)))
>>>>            return repo[n], replacements
>>>>    +class _multifold(fold):
>>>> +    """fold subclass used for when multiple folds happen in a row
>>>> +
>>>> +    We only want to fire the editor for the folded message once when
>>>> +    (say) four CLs are folded down into a single change. This is
>>>> +    similar to rollup, but we should preserve both messages so that
>>>> +    when the last fold operation runs we can show the user all the
>>>> +    commit messages in their editor.
>>>> +    """
>>>> +    def skipprompt(self):
>>>> +        return True
>>>> +
>>>>    class rollup(fold):
>>>> +    def mergedescs(self):
>>>> +        return False
>>>> +
>>>>        def skipprompt(self):
>>>>            return True
>>>>    @@ -644,6 +662,7 @@ actiontable = {'p': pick,
>>>>                   'edit': edit,
>>>>                   'f': fold,
>>>>                   'fold': fold,
>>>> +               '_multifold': _multifold,
>>>>                   'r': rollup,
>>>>                   'roll': rollup,
>>>>                   'd': drop,
>>>> @@ -850,6 +869,14 @@ def _histedit(ui, repo, state, *freeargs
>>>>                                            'histedit')
>>>>            state.backupfile = backupfile
>>>>    +    # preprocess rules so that we can hide inner folds from the user
>>>> +    # and only show one editor
>>>> +    rules = state.rules[:]
>>>> +    for idx, ((action, ha), (nextact, unused)) in enumerate(
>>>> +            zip(rules, rules[1:] + [(None, None)])):
>>>> +        if action == 'fold' and nextact == 'fold':
>>>> +            state.rules[idx] = '_multifold', ha
>>>
>>> This is moving us a bit back towards the world where there are hard coded
>>> special cases all over the place.  Can we add a preprocess() function to
>>> each histeditaction class and put this logic inside the multifold class?
>>> Then just call preprocess on every action class.
>>>
>>>> +
>>>>        while state.rules:
>>>>            state.write()
>>>>            action, ha = state.rules.pop(0)
>>>> @@ -995,7 +1022,7 @@ def verifyrules(rules, repo, ctxs):
>>>>                raise util.Abort(_('duplicated command for changeset %s')
>>>> %
>>>>                        ha[:12])
>>>>            seen.add(ha)
>>>> -        if action not in actiontable:
>>>> +        if action not in actiontable or action.startswith('_'):
>>>>                raise util.Abort(_('unknown action "%s"') % action)
>>>>            parsed.append([action, ha])
>>>>        missing = sorted(expected - seen)  # sort to stabilize output
>>>> diff --git a/tests/test-histedit-fold.t b/tests/test-histedit-fold.t
>>>> --- a/tests/test-histedit-fold.t
>>>> +++ b/tests/test-histedit-fold.t
>>>> @@ -509,4 +509,44 @@ into the hook command.
>>>>      $ hg add amended.txt
>>>>      $ hg ci -q --config extensions.largefiles= --amend -I amended.txt
>>>>    +  $ echo foo >> foo
>>>> +  $ hg add foo
>>>> +  $ hg ci -m foo1
>>>> +  $ echo foo >> foo
>>>> +  $ hg ci -m foo2
>>>> +  $ echo foo >> foo
>>>> +  $ hg ci -m foo3
>>>> +  $ hg logt
>>>> +  4:21679ff7675c foo3
>>>> +  3:b7389cc4d66e foo2
>>>> +  2:0e01aeef5fa8 foo1
>>>> +  1:578c7455730c a
>>>> +  0:79b99e9c8e49 b
>>>> +  $ cat > $TESTTMP/editor.sh <<EOF
>>>> +  > echo merged foos > $$1
>>>> +  > echo ran editor >> $TESTTMP/editorlog.txt
>>>> +  > EOF
>>>> +  $ HGEDITOR="sh $TESTTMP/editor.sh" hg histedit 1 --commands - 2>&1
>>>> <<EOF | fixbundle
>>>> +  > pick 578c7455730c 1 a
>>>> +  > pick 0e01aeef5fa8 2 foo1
>>>> +  > fold b7389cc4d66e 3 foo2
>>>> +  > fold 21679ff7675c 4 foo3
>>>> +  > EOF
>>>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>>> +  reverting foo
>>>> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>>>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>>> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>>> +  merging foo
>>>> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>>>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>>> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>>> +  $ hg logt
>>>> +  2:f7c59a6ef880 foo1
>>>> +  1:578c7455730c a
>>>> +  0:79b99e9c8e49 b
>>>> +Editor should have run only once
>>>> +  $ cat $TESTTMP/editorlog.txt
>>>> +  ran editor
>>>> +
>>>>      $ cd ..
>>>> _______________________________________________
>>>> Mercurial-devel mailing list
>>>> Mercurial-devel at selenic.com
>>>> https://selenic.com/mailman/listinfo/mercurial-devel
>>>
>>>
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel at selenic.com
>>> https://selenic.com/mailman/listinfo/mercurial-devel
>
>


More information about the Mercurial-devel mailing list