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

[Augie Fackler]

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