This is an archive of the discontinued Mercurial Phabricator instance.

Differential D934

rebase: add an experimental.showhashchanges config to show hash changes
AbandonedPublic

Authored by pulkit on Oct 4 2017, 10:34 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

This patch adds an experimental config, which if turned on will show hash
changes after a rebase. Support will be added to more commands which changes
hash.

This patch also adds test demonstrating the hash change output.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Oct 4 2017, 10:34 AM
pulkit updated this revision to Diff 2419.Oct 4 2017, 11:13 AM
durin42 added a subscriber: durin42.Oct 5 2017, 11:09 AM

I see what it does, but why? What's the motivation for the feature?

dlax added a subscriber: dlax.Oct 5 2017, 11:34 AM
pulkit added a comment.Oct 5 2017, 1:23 PM
In D934#15939, @durin42 wrote:

I see what it does, but why? What's the motivation for the feature?

It's for automation and helping editors such as Nuclide.

durin42 added a comment.Oct 5 2017, 1:24 PM
In D934#15995, @pulkit wrote:
In D934#15939, @durin42 wrote:

I see what it does, but why? What's the motivation for the feature?

It's for automation and helping editors such as Nuclide.

Could you please put that in the commit message?

I wonder if we could make this stuff that was just --verbose output for rebase, and always put it in the json output (-Tjson) via the formatter, and not require a config. What do you think of taht approach?

quark added a subscriber: quark.Oct 5 2017, 2:34 PM

I'm trying to write a version that decouples the rendering logic from the data. But it's trickier than I thought (ex. issue5699). I guess some refactoring needs to be done. Meanwhile I think it would be good enough if you can avoid passing fm around.

hgext/rebase.py
556

I think it's unnecessary to have a config option - there is no behavior change if users use rebase as before.

560

Instead of passing fm around, it would be cleaner if the content could be generated, like:

# "fm" is not passed elsewhere
with ui.formatter('rebase', opts) as fm:
    fm.startitem()
    fm.data(nodechanges=calculatenodechanges(mapping))

(node seems to be more consistent with existing terms than hash)

But this might be difficult without the args templatekeywords context.

tests/test-rebase-base.t
418

The -base.t test was intended for testing --base flag. We probably want a new .t file like test-rebase-templates.t.

462

For JSON output, it's usually for automation and better to use full hashes.

pulkit planned changes to this revision.Oct 6 2017, 5:15 PM
pulkit added a comment.Oct 17 2017, 7:14 PM
In D934#15996, @durin42 wrote:
In D934#15995, @pulkit wrote:
In D934#15939, @durin42 wrote:

I see what it does, but why? What's the motivation for the feature?

It's for automation and helping editors such as Nuclide.

Could you please put that in the commit message?
I wonder if we could make this stuff that was just --verbose output for rebase, and always put it in the json output (-Tjson) via the formatter, and not require a config. What do you think of taht approach?

I like the approach but I have followed Jun's one as that was easy and no UI change. :)

pulkit abandoned this revision.Oct 17 2017, 7:14 PM

Abandoning in favor of D1173.

Revision Contents
Changeset List

PathPackages
Mhgext/rebase.py (15 lines)
Mmercurial/configitems.py (3 lines)
Mtests/test-rebase-base.t (46 lines)

Diff 2419

hgext/rebase.py

if newwd not in [c.rev() for c in repo[None].parents()]: if newwd not in [c.rev() for c in repo[None].parents()]:
ui.note(_("update back to initial working directory parent\n")) ui.note(_("update back to initial working directory parent\n"))
hg.updaterepo(repo, newwd, False) hg.updaterepo(repo, newwd, False)
collapsedas = None collapsedas = None
if not self.keepf: if not self.keepf:
if self.collapsef: if self.collapsef:
collapsedas = newnode collapsedas = newnode
fm = None
if repo.ui.configbool('experimental', 'showhashchanges'):
quarkUnsubmitted
Not Done

I think it's unnecessary to have a config option - there is no behavior change if users use rebase as before.

quark: I think it's unnecessary to have a config option - there is no behavior change if users use…
fm = repo.ui.formatter('rebase', opts)
clearrebased(ui, repo, self.destmap, self.state, self.skipped, clearrebased(ui, repo, self.destmap, self.state, self.skipped,
collapsedas, self.keepf) collapsedas, self.keepf, fm=fm)
quarkUnsubmitted
Not Done

Instead of passing fm around, it would be cleaner if the content could be generated, like:

# "fm" is not passed elsewhere
with ui.formatter('rebase', opts) as fm:
    fm.startitem()
    fm.data(nodechanges=calculatenodechanges(mapping))

(node seems to be more consistent with existing terms than hash)

But this might be difficult without the args templatekeywords context.

quark: Instead of passing `fm` around, it would be cleaner if the content could be generated, like…
if fm:
fm.end()
clearstatus(repo) clearstatus(repo)
clearcollapsemsg(repo) clearcollapsemsg(repo)
ui.note(_("rebase completed\n")) ui.note(_("rebase completed\n"))
util.unlinkpath(repo.sjoin('undo'), ignoremissing=True) util.unlinkpath(repo.sjoin('undo'), ignoremissing=True)
if self.skipped: if self.skipped:
skippedlen = len(self.skipped) skippedlen = len(self.skipped)
for rev in sorted(state): for rev in sorted(state):
parents = [p for p in repo.changelog.parentrevs(rev) if p != nullrev] parents = [p for p in repo.changelog.parentrevs(rev) if p != nullrev]
# if all parents of this revision are done, then so is this revision # if all parents of this revision are done, then so is this revision
if parents and all((state.get(p) == p for p in parents)): if parents and all((state.get(p) == p for p in parents)):
state[rev] = rev state[rev] = rev
return originalwd, destmap, state return originalwd, destmap, state
def clearrebased(ui, repo, destmap, state, skipped, collapsedas=None, def clearrebased(ui, repo, destmap, state, skipped, collapsedas=None,
keepf=False): keepf=False, fm=None):
"""dispose of rebased revision at the end of the rebase """dispose of rebased revision at the end of the rebase
If `collapsedas` is not None, the rebase was a collapse whose result if the If `collapsedas` is not None, the rebase was a collapse whose result if the
`collapsedas` node. `collapsedas` node.
If `keepf` is not True, the rebase has --keep set and no nodes should be If `keepf` is not True, the rebase has --keep set and no nodes should be
removed (but bookmarks still need to be moved). removed (but bookmarks still need to be moved).
""" """
tonode = repo.changelog.node tonode = repo.changelog.node
replacements = {} replacements = {}
moves = {} moves = {}
for rev, newrev in sorted(state.items()): for rev, newrev in sorted(state.items()):
if newrev >= 0 and newrev != rev: if newrev >= 0 and newrev != rev:
oldnode = tonode(rev) oldnode = tonode(rev)
newnode = collapsedas or tonode(newrev) newnode = collapsedas or tonode(newrev)
moves[oldnode] = newnode moves[oldnode] = newnode
if not keepf: if not keepf:
if rev in skipped: if rev in skipped:
succs = () succs = ()
else: else:
succs = (newnode,) succs = (newnode,)
replacements[oldnode] = succs replacements[oldnode] = succs
scmutil.cleanupnodes(repo, replacements, 'rebase', moves) scmutil.cleanupnodes(repo, replacements, 'rebase', moves)
if fm:
fm.startitem()
scmutil.showchanges(replacements, fm)
def pullrebase(orig, ui, repo, *args, **opts): def pullrebase(orig, ui, repo, *args, **opts):
'Call rebase after pull if the latter has been invoked with --rebase' 'Call rebase after pull if the latter has been invoked with --rebase'
ret = None ret = None
if opts.get('rebase'): if opts.get('rebase'):
if ui.configbool('commands', 'rebase.requiredest'): if ui.configbool('commands', 'rebase.requiredest'):
msg = _('rebase destination required by configuration') msg = _('rebase destination required by configuration')
hint = _('use hg pull followed by hg rebase -d DEST') hint = _('use hg pull followed by hg rebase -d DEST')

mercurial/configitems.py

default=False, default=False,
) )
coreconfigitem('experimental', 'revertalternateinteractivemode', coreconfigitem('experimental', 'revertalternateinteractivemode',
default=True, default=True,
) )
coreconfigitem('experimental', 'revlogv2', coreconfigitem('experimental', 'revlogv2',
default=None, default=None,
) )
coreconfigitem('experimental', 'showhashchanges',
default=False,
)
coreconfigitem('experimental', 'spacemovesdown', coreconfigitem('experimental', 'spacemovesdown',
default=False, default=False,
) )
coreconfigitem('experimental', 'treemanifest', coreconfigitem('experimental', 'treemanifest',
default=False, default=False,
) )
coreconfigitem('experimental', 'updatecheck', coreconfigitem('experimental', 'updatecheck',
default=None, default=None,

tests/test-rebase-base.t

o 3: B o 3: B
| |
o 2: Z o 2: Z
| |
o 1: Y o 1: Y
| |
o 0: A o 0: A
Enabling obsolete markers
quarkUnsubmitted
Not Done

The -base.t test was intended for testing --base flag. We probably want a new .t file like test-rebase-templates.t.

quark: The `-base.t` test was intended for testing `--base` flag. We probably want a new `.t` file…
$ cat >> $HGRCPATH << EOF
> [experimental]
> stabilization=createmarkers
> showhashchanges=True
> EOF
Testing the experimental.showhashchanges config
$ hg rebase -s 4 -d 2
rebasing 4:abc67d0cf023 "C"
rebasing 5:a701fddfacec "D" (tip)
'a701fddfacec' --> ['003ba25ccc56']
'abc67d0cf023' --> ['2e4b11ea9404']
$ hg tglog
o 7: D
|
o 6: C
|
| o 3: B
|/
o 2: Z
|
o 1: Y
|
o 0: A
JSON output using the config
$ hg rebase -s 6 -d 3 -Tjson
rebasing 6:2e4b11ea9404 "C"
rebasing 7:003ba25ccc56 "D" (tip)
[
{
"hashchanges": {"003ba25ccc56": ["38bd5f90ba6a"], "2e4b11ea9404": ["583565ab89ac"]}
}
]
Using `-q` to make sure ui.status() things don't appear
$ hg rebase -s 8 -d 2 -T '{hashchanges|json}' -q
{"38bd5f90ba6a": ["28cb79571ec7"], "583565ab89ac": ["89a178738706"]} (no-eol)
quarkUnsubmitted
Not Done

For JSON output, it's usually for automation and better to use full hashes.

quark: For JSON output, it's usually for automation and better to use full hashes.
$ cd .. $ cd ..