def _dryrunrebase(ui, repo, opts):

rbsrt = rebaseruntime(repo, ui, inmemory=True, opts=opts)

• ui.status(_('starting dry-run rebase; repository will not be changed\n'))

+ confirm = opts.get('confirm')
+ if confirm:
+ ui.status(_('starting rebase...\n'))
+ else:
+ ui.status(_('starting dry-run rebase; repository will not be '
+ 'changed\n'))

Nit: Perhaps --dry-run should precede --confirm, or "--dryrun --confirm" should
be rejected.

with repo.wlock(), repo.lock():
    try:
        overrides = {('rebase', 'singletransaction'): True}
        with ui.configoverride(overrides, 'rebase'):
            _origrebase(ui, repo, opts, rbsrt, inmemory=True,
                        leaveunfinished=True, supptrwarns=True)
    except error.InMemoryMergeConflictsError:

+ conflict = True

    ui.status(_('hit a merge conflict\n'))
    return 1
else:

• ui.status(_('dry-run rebase completed successfully; run without '
• '-n/--dry-run to perform this rebase\n'))

+ conflict = False
+ if confirm:
+ ui.status(_('rebase completed successfully\n'))
+ else:
+ ui.status(_('dry-run rebase completed successfully; run without'
+ ' -n/--dry-run to perform this rebase\n'))

    return 0
finally:
    # no need to store backup in case of dryrun
    rbsrt._prepareabortorcontinue(isabort=True, backup=False,
                                  suppwarns=True)

+ if confirm:
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):
+ if not conflict:
+ inmemory = ui.configbool('rebase',
+ 'experimental.inmemory')
+ return _dorebase(ui, repo, opts, inmemory=inmemory)
+ return _dorebase(ui, repo, opts)

Can't we just "finish" the last in-memory rebase if "apply changes" selected?

In D3870#60813, @yuja wrote:

def _dryrunrebase(ui, repo, opts):

rbsrt = rebaseruntime(repo, ui, inmemory=True, opts=opts)

• ui.status(_('starting dry-run rebase; repository will not be changed\n'))

+ confirm = opts.get('confirm')
+ if confirm:
+ ui.status(_('starting rebase...\n'))
+ else:
+ ui.status(_('starting dry-run rebase; repository will not be '
+ 'changed\n'))

Nit: Perhaps --dry-run should precede --confirm, or "--dryrun --confirm" should
be rejected.

I think later option is good.

with repo.wlock(), repo.lock():
    try:
        overrides = {('rebase', 'singletransaction'): True}
        with ui.configoverride(overrides, 'rebase'):
            _origrebase(ui, repo, opts, rbsrt, inmemory=True,
                        leaveunfinished=True, supptrwarns=True)
    except error.InMemoryMergeConflictsError:

+ conflict = True

    ui.status(_('hit a merge conflict\n'))
    return 1
else:

• ui.status(_('dry-run rebase completed successfully; run without '
• '-n/--dry-run to perform this rebase\n'))

+ conflict = False
+ if confirm:
+ ui.status(_('rebase completed successfully\n'))
+ else:
+ ui.status(_('dry-run rebase completed successfully; run without'
+ ' -n/--dry-run to perform this rebase\n'))

    return 0
finally:
    # no need to store backup in case of dryrun
    rbsrt._prepareabortorcontinue(isabort=True, backup=False,
                                  suppwarns=True)

+ if confirm:
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):
+ if not conflict:
+ inmemory = ui.configbool('rebase',
+ 'experimental.inmemory')
+ return _dorebase(ui, repo, opts, inmemory=inmemory)
+ return _dorebase(ui, repo, opts)

Can't we just "finish" the last in-memory rebase if "apply changes" selected?

Ah, I think we can :)
will update it, thanks!

This can't be applied, can you rebase?

• a/hgext/rebase.py

+++ b/hgext/rebase.py
@@ -677,7 +677,7 @@

('a', 'abort', False, _('abort an interrupted rebase')),
('', 'auto-orphans', '', _('automatically rebase orphan revisions '
                           'in the specified revset (EXPERIMENTAL)')),

• ] + cmdutil.dryrunopts + cmdutil.formatteropts,

+ ] + cmdutil.dryrunopts + cmdutil.formatteropts + cmdutil.confirmopts,

_('[-s REV | -b REV] [-d REV] [OPTION]'))

def rebase(ui, repo, **opts):

"""move changeset (and descendants) to a different branch

@@ -808,6 +808,10 @@

    raise error.Abort(_('cannot specify both --dry-run and --abort'))
if opts.get('continue'):
    raise error.Abort(_('cannot specify both --dry-run and --continue'))

+ if opts.get('confirm'):
+ raise error.Abort(_('cannot specify both --dry-run and --confirm'))
+ if opts.get('confirm'):
+ dryrun = True

Nit: we'll have to reject "--confirm --abort" and "--confirm --continue" as
well.

with repo.wlock(), repo.lock():
    try:
        overrides = {('rebase', 'singletransaction'): True}
        with ui.configoverride(overrides, 'rebase'):
            _origrebase(ui, repo, opts, rbsrt, inmemory=True,
                        leaveunfinished=True, supptrwarns=True)
    except error.InMemoryMergeConflictsError:

+ conflict = True

    ui.status(_('hit a merge conflict\n'))
    return 1
else:

• ui.status(_('dry-run rebase completed successfully; run without '
• '-n/--dry-run to perform this rebase\n'))

+ conflict = False
+ if confirm:
+ ui.status(_('rebase completed successfully\n'))
+ else:
+ ui.status(_('dry-run rebase completed successfully; run without'
+ ' -n/--dry-run to perform this rebase\n'))

    return 0
finally:

• # no need to store backup in case of dryrun
• rbsrt._prepareabortorcontinue(isabort=True, backup=False,
• suppwarns=True)

+ if confirm and not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):
+ if conflict:
+ rbsrt._prepareabortorcontinue(isabort=True,
+ backup=False,
+ suppwarns=True)
+ _dorebase(ui, repo, opts, inmemory=False)
+ else:
+ # finish unfinished rebase
+ rbsrt._finishrebase()

I'm not a fan of doing much things "finally:" clause. Can you move this to
the "else:" clause? The "conflict" flag can be flipped to be a "needsabort",
and it has to be initialized before "try:".

@yuja Rebased, removed variable "needsabort/conflict" and moved that code to "except" and "else" clause accordingly.

Still can't apply. Which revision is this patch based on?

with repo.wlock(), repo.lock():
    try:
        overrides = {('rebase', 'singletransaction'): True}
        with ui.configoverride(overrides, 'rebase'):
            _origrebase(ui, repo, opts, rbsrt, inmemory=True,
                        leaveunfinished=True, supptrwarns=True)
    except error.InMemoryMergeConflictsError:
        ui.status(_('hit a merge conflict\n'))

+ if confirm:
+ # abort as in-memory merge doesn't support conflict
+ rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+ suppwarns=True)
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):
+ _dorebase(ui, repo, opts, inmemory=False)

    return 1
else:

• ui.status(_('dry-run rebase completed successfully; run without '
• '-n/--dry-run to perform this rebase\n'))

+ if confirm:
+ ui.status(_('rebase completed successfully\n'))
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):

If KeyboardInterrupt is raised here for example, abort() wouldn't be called.

finally:

• # no need to store backup in case of dryrun
• rbsrt._prepareabortorcontinue(isabort=True, backup=False,
• suppwarns=True)

+ if not confirm:
+ # no need to store backup in case of dryrun
+ rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+ suppwarns=True)

In D3870#61079, @yuja wrote:

Still can't apply. Which revision is this patch based on?

Ah, this patch is based on https://phab.mercurial-scm.org/D3830 which is not reviewed. Do I need to rebase this to hg-commited current tip?

with repo.wlock(), repo.lock():
    try:
        overrides = {('rebase', 'singletransaction'): True}
        with ui.configoverride(overrides, 'rebase'):
            _origrebase(ui, repo, opts, rbsrt, inmemory=True,
                        leaveunfinished=True, supptrwarns=True)
    except error.InMemoryMergeConflictsError:
        ui.status(_('hit a merge conflict\n'))

+ if confirm:
+ # abort as in-memory merge doesn't support conflict
+ rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+ suppwarns=True)
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):
+ _dorebase(ui, repo, opts, inmemory=False)

    return 1
else:

• ui.status(_('dry-run rebase completed successfully; run without '
• '-n/--dry-run to perform this rebase\n'))

+ if confirm:
+ ui.status(_('rebase completed successfully\n'))
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):

If KeyboardInterrupt is raised here for example, abort() wouldn't be called.

okay, can I update this patch or send a follow-up to handle KeyboardInterrupt?

finally:

• # no need to store backup in case of dryrun
• rbsrt._prepareabortorcontinue(isabort=True, backup=False,
• suppwarns=True)

+ if not confirm:
+ # no need to store backup in case of dryrun
+ rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+ suppwarns=True)

Ah, this patch is based on https://phab.mercurial-scm.org/D3830 which is
not reviewed. Do I need to rebase this to hg-commited current tip?

Can you rewrite this to not depend on D3830 if that's easy? It appears
nobody reviewed D3830 yet, and I can't trivially say it's good to go.

> If KeyboardInterrupt is raised here for example, abort() wouldn't be called.
okay, can I update this patch or send a follow-up to handle KeyboardInterrupt?

Can you fix that in this patch? KeyboardInterrupt is just an example. The
abort in the finally clause can't be gated by the confirm flag.

Queued, thanks.

• ui.status(_('starting dry-run rebase; repository will not be changed\n'))

+ confirm = opts.get('confirm')
+ if confirm:
+ ui.status(_('starting rebase...\n'))

Nit: this message is misleading since nothing will be committed until
accepting the changes. Can you remove it? Or alternatively, we can say that
you're doing rebase in-memory so you're safe.

with repo.wlock(), repo.lock():
    try:

+ needsabort = True

Moved this before the "try".

except error.InMemoryMergeConflictsError:
    ui.status(_('hit a merge conflict\n'))

+ if confirm:
+ # abort as in-memory merge doesn't support conflict
+ rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+ suppwarns=True)
+ needsabort = False
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):

Nit: This isn't actually "apply changes".

+ _dorebase(ui, repo, opts, inmemory=False)

return _dorebase(...) to not loose the actual result code. Can you
fix by a follow-up patch?

except error.InMemoryMergeConflictsError:
    ui.status(_('hit a merge conflict\n'))

+ if confirm:
+ # abort as in-memory merge doesn't support conflict
+ rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+ suppwarns=True)
+ needsabort = False
+ if not ui.promptchoice(_(b'apply changes (yn)?'
+ b'$$ &Yes $$ &No')):

Nit: This isn't actually "apply changes".

Hmm, then what it should be?

>> +            if confirm:
>>  +                # abort as in-memory merge doesn't support conflict
>>  +                rbsrt._prepareabortorcontinue(isabort=True, backup=False,
>>  +                                              suppwarns=True)
>>  +                needsabort = False
>>  +                if not ui.promptchoice(_(b'apply changes (yn)?'
>>  +                                         b'$$ &Yes $$ &No')):
> 
> Nit: This isn't actually "apply changes".
Hmm, then what it should be?

I think we'll need to tell that we'll rerun the rebase that's known to have
at least one merge conflict. "apply changes" sounds like it would just
finalize the rebase up to the previous revision of the merge conflict.

Alternatively, it can just abort without the prompt since the dry-run was
incomplete.