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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
Mmercurial/filemerge.py (19 lines)

Diff 32190

mercurial/filemerge.py

templatekw, templatekw,
templater, templater,
templateutil, templateutil,
util, util,
) )
from .utils import ( from .utils import (
procutil, procutil,
stringutil,
) )
def _toolstr(ui, tool, part, *args): def _toolstr(ui, tool, part, *args):
return ui.config(b"merge-tools", tool + b"." + part, *args) return ui.config(b"merge-tools", tool + b"." + part, *args)
def _toolbool(ui, tool, part, *args): def _toolbool(ui, tool, part, *args):
underyling workingfilectx in that case. underyling workingfilectx in that case.
""" """
if filectx.isabsent(): if filectx.isabsent():
return filectx.changectx()[filectx.path()] return filectx.changectx()[filectx.path()]
else: else:
return filectx return filectx
def _verifytext(input, ui):
"""verifies that text is non-binary"""
if stringutil.binary(input.text()):
msg = _(b"%s looks like a binary file.") % input.fctx.path()
ui.warn(_(b'warning: %s\n') % msg)
raise error.Abort(msg)
def _premerge(repo, local, other, base, toolconf, backup): def _premerge(repo, local, other, base, toolconf, backup):
tool, toolpath, binary, symlink, scriptfn = toolconf tool, toolpath, binary, symlink, scriptfn = toolconf
if symlink or local.fctx.isabsent() or other.fctx.isabsent(): if symlink or local.fctx.isabsent() or other.fctx.isabsent():
return 1 return 1
ui = repo.ui ui = repo.ui
validkeep = [b'keep', b'keep-merge3', b'keep-mergediff'] validkeep = [b'keep', b'keep-merge3', b'keep-mergediff']
) )
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'
if any(
stringutil.binary(input.text()) for input in (local, base, other)
):
return 1 # continue merging
r = simplemerge.simplemerge( r = simplemerge.simplemerge(
ui, local, base, other, quiet=True, mode=mode ui, local, base, other, quiet=True, 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
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:
_verifytext(local, ui)
_verifytext(base, ui)
_verifytext(other, ui)
except error.Abort:
return True, True, False
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,
_( _(