D3757: rebase: add dry-run functionality

Yuya Nishihara yuya at tcha.org
Sun Jun 17 00:34:45 EDT 2018


Just nitpicking. The feature generally looks good to me.

> @@ -798,6 +797,15 @@
>  
>      """
>      inmemory = ui.configbool('rebase', 'experimental.inmemory')
> +    dryrun = opts.get(r'dry_run')
> +    if dryrun:
> +        if (opts.get(r'abort')):
> +                raise error.Abort(_('cannot specify both --dry-run '
> +                                    'and --abort'))
> +        if (opts.get(r'continue')):
> +                raise error.Abort(_('cannot specify both --dry-run '
> +                                    'and --continue'))
           ^^^^^^^^
excessive indent and unnecessary parens.

> -    if inmemory:
> +    if dryrun:
>          try:
> -            # in-memory merge doesn't support conflicts, so if we hit any, abort
> -            # and re-run as an on-disk merge.
>              overrides = {('rebase', 'singletransaction'): True}
>              with ui.configoverride(overrides, 'rebase'):
> -                return _origrebase(ui, repo, inmemory=inmemory, **opts)
> +                _origrebase(ui, repo, inmemory=True, dryrun=dryrun, **opts)
                                                        ^^^^^^^^^^^^^

Perhaps this flag shouldn't be called a `dryrun` because it would leave
a rebase session unless we do abort.

>          except error.InMemoryMergeConflictsError:
> -            ui.warn(_('hit merge conflicts; re-running rebase without in-memory'
> -                      ' merge\n'))
> +            ui.status(_('Hit a merge conflict\n'))
> +        else:
> +            ui.status(_('There will be no conflict, you can rebase\n'))
> +        finally:
>              _origrebase(ui, repo, **{r'abort': True})
                                     ^^^^^^^^^^^^^^^^^^^

It can be written as just `abort=True`.

> -            return _origrebase(ui, repo, inmemory=False, **opts)
> +
>      else:
> -        return _origrebase(ui, repo, **opts)
> +        if inmemory:

I slightly prefer `elif inmemory` than nesting one more depth.


More information about the Mercurial-devel mailing list