This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12147

simplemerge: let filemerge check for binary inputs
ClosedPublic

Authored by martinvonz on Feb 8 2022, 4:26 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGd9af7c1fb619: simplemerge: let filemerge check for binary inputs
Summary

This is similar to the previous patch, but here we put a specialized
copy of simplemerge._verifytext() in the the filemerge module
instead.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Feb 8 2022, 4:26 PM
Alphare added a subscriber: Alphare.Feb 9 2022, 11:04 AM

I am not entirely sure if removing the check from simplemerge is a good idea, new callers would need to think to verify the text beforehand. Maybe a _verified attribute could be used to skip the check if already done?

martinvonz planned changes to this revision.Feb 9 2022, 11:23 AM
In D12147#186335, @Alphare wrote:

I am not entirely sure if removing the check from simplemerge is a good idea, new callers would need to think to verify the text beforehand. Maybe a _verified attribute could be used to skip the check if already done?

Yes, that's fair. I'll refactor this in some way. I'd still like to remove the ui argument from simplemerge() because it seems like a library function to me (though I know we don't separate between library and UI almost anywhere :)).

martinvonz added a parent revision: D12146: simplemerge: let extension check for binary inputs (unless `--text`).Feb 10 2022, 9:55 PM
martinvonz retitled this revision from simplemerge: let users call verifytext() to simplemerge: let filemerge check for binary inputs.
martinvonz edited the summary of this revision. (Show Details)
martinvonz added a child revision: D12169: simplemerge: remove code for checking binary input now that callers do it.
martinvonz updated this revision to Diff 32136.
Alphare accepted this revision.Feb 14 2022, 5:40 AM
This revision is now accepted and ready to land.Feb 14 2022, 5:40 AM
martinvonz added a commit: rHGd9af7c1fb619: simplemerge: let filemerge check for binary inputs.Feb 15 2022, 7:56 AM
Closed by commit rHGd9af7c1fb619: simplemerge: let filemerge check for binary inputs (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcontrib/simplemerge (15 lines)
Mmercurial/filemerge.py (20 lines)
Mmercurial/simplemerge.py (10 lines)

Diff 32096

contrib/simplemerge

context.arbitraryfilectx(local), labels[0] context.arbitraryfilectx(local), labels[0]
) )
other_input = simplemerge.MergeInput( other_input = simplemerge.MergeInput(
context.arbitraryfilectx(other), labels[1] context.arbitraryfilectx(other), labels[1]
) )
base_input = simplemerge.MergeInput( base_input = simplemerge.MergeInput(
context.arbitraryfilectx(base), labels[2] context.arbitraryfilectx(base), labels[2]
) )
ui = uimod.ui.load()
opts = pycompat.strkwargs(opts)
try:
local_input.verifytext(ui, **opts)
base_input.verifytext(ui, **opts)
other_input.verifytext(ui, **opts)
except error.Abort:
sys.exit(1)
sys.exit( sys.exit(
simplemerge.simplemerge( simplemerge.simplemerge(
uimod.ui.load(), ui, local_input, base_input, other_input, **opts
local_input,
base_input,
other_input,
**pycompat.strkwargs(opts)
) )
) )
except ParseError as e: except ParseError as e:
e = stringutil.forcebytestr(e) e = stringutil.forcebytestr(e)
procutil.stdout.write(b"%s: %s\n" % (sys.argv[0].encode('utf8'), e)) procutil.stdout.write(b"%s: %s\n" % (sys.argv[0].encode('utf8'), e))
showhelp() showhelp()
sys.exit(1) sys.exit(1)
except error.Abort as e: except error.Abort as e:
procutil.stderr.write(b"abort: %s\n" % e) procutil.stderr.write(b"abort: %s\n" % e)
sys.exit(255) sys.exit(255)
except KeyboardInterrupt: except KeyboardInterrupt:
sys.exit(255) sys.exit(255)

mercurial/filemerge.py

) )
if premerge: if premerge:
mode = b'merge' mode = b'merge'
if premerge == b'keep-mergediff': if premerge == b'keep-mergediff':
mode = b'mergediff' mode = b'mergediff'
elif premerge == b'keep-merge3': elif premerge == b'keep-merge3':
mode = b'merge3' mode = b'merge3'
r = simplemerge.simplemerge( try:
ui, local, base, other, quiet=True, mode=mode local.verifytext(ui, quiet=True)
) base.verifytext(ui, quiet=True)
other.verifytext(ui, quiet=True)
except error.Abort:
r = True
else:
r = simplemerge.simplemerge(ui, local, base, other, mode=mode)
if not r: if not r:
ui.debug(b" premerge successful\n") ui.debug(b" premerge successful\n")
return 0 return 0
if premerge not in validkeep: if premerge not in validkeep:
# restore from backup and try again # restore from backup and try again
_restorebackup(local.fctx, backup) _restorebackup(local.fctx, backup)
return 1 # continue merging return 1 # continue merging
def _merge(repo, local, other, base, mode): def _merge(repo, local, other, base, mode):
""" """
Uses the internal non-interactive simple merge algorithm for merging Uses the internal non-interactive simple merge algorithm for merging
files. It will fail if there are any conflicts and leave markers in files. It will fail if there are any conflicts and leave markers in
the partially merged file. Markers will have two sections, one for each side the partially merged file. Markers will have two sections, one for each side
of merge, unless mode equals 'union' which suppresses the markers.""" of merge, unless mode equals 'union' which suppresses the markers."""
ui = repo.ui ui = repo.ui
try:
local.verifytext(ui)
base.verifytext(ui)
other.verifytext(ui)
except error.Abort:
r = True
else:
r = simplemerge.simplemerge(ui, local, base, other, mode=mode) r = simplemerge.simplemerge(ui, local, base, other, mode=mode)
return True, r, False return True, r, False
@internaltool( @internaltool(
b'union', b'union',
fullmerge, fullmerge,
_( _(
b"warning: conflicts while merging %s! " b"warning: conflicts while merging %s! "

mercurial/simplemerge.py

then we just warn)""" then we just warn)"""
text = self.text() text = self.text()
if stringutil.binary(text): if stringutil.binary(text):
msg = _(b"%s looks like a binary file.") % self.fctx.path() msg = _(b"%s looks like a binary file.") % self.fctx.path()
if not opts.get('quiet'): if not opts.get('quiet'):
ui.warn(_(b'warning: %s\n') % msg) ui.warn(_(b'warning: %s\n') % msg)
if not opts.get('text'): if not opts.get('text'):
raise error.Abort(msg) raise error.Abort(msg)
return text
def simplemerge(ui, local, base, other, **opts): def simplemerge(ui, local, base, other, **opts):
"""Performs the simplemerge algorithm. """Performs the simplemerge algorithm.
The merged result is written into `localctx`. The merged result is written into `localctx`.
""" """
try: m3 = Merge3Text(base.text(), local.text(), other.text())
localtext = local.verifytext(ui, **opts)
basetext = base.verifytext(ui, **opts)
othertext = other.verifytext(ui, **opts)
except error.Abort:
return True
m3 = Merge3Text(basetext, localtext, othertext)
conflicts = False conflicts = False
mode = opts.get('mode', b'merge') mode = opts.get('mode', b'merge')
if mode == b'union': if mode == b'union':
lines = _resolve(m3, (1, 2)) lines = _resolve(m3, (1, 2))
elif mode == b'local': elif mode == b'local':
lines = _resolve(m3, (1,)) lines = _resolve(m3, (1,))
elif mode == b'other': elif mode == b'other':
lines = _resolve(m3, (2,)) lines = _resolve(m3, (2,))