( )⚙ D2889 filemerge: make the 'local' path match the format that 'base' and 'other' use

This is an archive of the discontinued Mercurial Phabricator instance.

filemerge: make the 'local' path match the format that 'base' and 'other' use
ClosedPublic

Authored by spectral on Mar 16 2018, 8:00 PM.

Details

Summary

If we pass a separate '$output' arg to the merge tool, we produce four files:
local, base, other, and output. In this situation, 'output' will be the
original filename, 'base' and 'other' are temporary files, and previously
'local' would be the backup file (so if 'output' was foo.txt, 'local' would be
foo.txt.orig).

This change makes it so that 'local' follows the same pattern as 'base' and
'other' - it will be a temporary file either in the
experimental.mergetempdirprefix-controlled directory with a name like
foo~local.txt, or in the normal system-wide temp dir with a name like
foo~local.RaNd0m.txt.

For the cases where the merge tool does not use an '$output' arg, 'local' is
still the destination filename, and 'base' and 'other' are unchanged.

The hope is that this is much easier for people to reason about; rather than
having a tool like Meld pop up with three panes, one of them with the filename
"foo.txt.orig", one with the filename "foo.txt", and one with
"foo~other.StuFf2.txt", we can (when the merge temp dir stuff is enabled) make
it show up as "foo~local.txt", "foo.txt" and "foo~other.txt", respectively.

This also opens the door to future customization, such as getting the
operation-provided labels and a hash prefix into the filenames (so we see
something like "foo~dest.abc123", "foo.txt", and "foo~src.d4e5f6").

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

spectral created this revision.Mar 16 2018, 8:00 PM
yuja requested changes to this revision.Mar 19 2018, 10:33 AM
yuja added a subscriber: yuja.

Not reviewed yet, but this can't be applied cleanly. Can you rebase?

This revision now requires changes to proceed.Mar 19 2018, 10:33 AM
In D2889#46482, @yuja wrote:

Not reviewed yet, but this can't be applied cleanly. Can you rebase?

This series should rebase cleanly onto a4a95bd7 (which was accepted by pulkit separately from the rest of the series). I've rebase these three remaining ones and pushed.

spectral requested review of this revision.Mar 19 2018, 2:22 PM

Ah, there's a "Request Review" that I'd never noticed before, choosing that to hopefully get this out of "Requires Revision"

yuja added a comment.Mar 21 2018, 12:23 AM

Can't apply.

% hg up -C a4a95bd7
2 files updated, 0 files merged, 0 files removed, 0 files unresolved

% hg pimp D2889
devel-warn: accessing unregistered config item: 'phabricator.batchsize' at: /home/yuya/work/hghacks/mercurial/mercurial/ui.py:624 (configwith)
devel-warn: accessing unregistered config item: 'phabricator.url' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:124 (readurltoken)
devel-warn: accessing unregistered config item: 'phabricator.token' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:111 (readlegacytoken)
phabricator.token is deprecated - please migrate to the phabricator.auth section.
devel-warn: accessing unregistered config item: 'phabricator.curlcmd' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:162 (callconduit)
applying patch from stdin
devel-warn: accessing unregistered config item: 'phabricator.url' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:124 (readurltoken)
devel-warn: accessing unregistered config item: 'phabricator.token' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:111 (readlegacytoken)
devel-warn: accessing unregistered config item: 'phabricator.curlcmd' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:162 (callconduit)
devel-warn: accessing unregistered config item: 'phabricator.url' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:124 (readurltoken)
devel-warn: accessing unregistered config item: 'phabricator.token' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:111 (readlegacytoken)
devel-warn: accessing unregistered config item: 'phabricator.curlcmd' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:162 (callconduit)
patching file tests/test-merge-tools.t
Hunk #2 FAILED at 1599
1 out of 2 hunks FAILED -- saving rejects to file tests/test-merge-tools.t.rej
patching file mercurial/filemerge.py
Hunk #1 FAILED at 511
Hunk #3 FAILED at 663
2 out of 3 hunks FAILED -- saving rejects to file mercurial/filemerge.py.rej
abort: patch failed to apply
spectral updated this revision to Diff 7189.Mar 21 2018, 4:48 PM
yuja accepted this revision.Mar 22 2018, 10:34 AM
This revision is now accepted and ready to land.Mar 22 2018, 10:34 AM
This revision was automatically updated to reflect the committed changes.