This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7464

filemerge: drop a default argument to appease pytype
ClosedPublic

Authored by mharbison72 on Nov 20 2019, 10:27 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG1ffbd03c8d75: filemerge: drop a default argument to appease pytype
Summary

The function slices and takes the length of this argument without internally
setting it if not provided. There was no bug here because both callers passed
the argument.

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

mharbison72 created this revision.Nov 20 2019, 10:27 PM
dlax added a subscriber: dlax.Nov 21 2019, 3:07 AM

Shouldn't this be also done for all similar functions? (i.e. _xmergeimm and functions registered as a merge tool with @internaltool)

mharbison72 added a comment.Dec 1 2019, 1:38 PM
In D7464#109795, @dlax wrote:

Shouldn't this be also done for all similar functions? (i.e. _xmergeimm and functions registered as a merge tool with @internaltool)

I looked at this a bit more, and it looks like the other functions either ignore the labels arg, or they handle the None/empty case to substitute something else. So maybe that's a better thing to do. But I'm not real familiar with merge code, and don't know what the default labels should be here.

mharbison72 added a commit: rHG1ffbd03c8d75: filemerge: drop a default argument to appease pytype.Dec 11 2019, 1:30 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG1ffbd03c8d75: filemerge: drop a default argument to appease pytype (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/filemerge.py (2 lines)

Diff 18619

mercurial/filemerge.py

# things for us to import cmdutil. # things for us to import cmdutil.
tres = formatter.templateresources(ui, repo) tres = formatter.templateresources(ui, repo)
t = formatter.maketemplater( t = formatter.maketemplater(
ui, tmpl, defaults=templatekw.keywords, resources=tres ui, tmpl, defaults=templatekw.keywords, resources=tres
) )
ui.status(t.renderdefault(props)) ui.status(t.renderdefault(props))
def _xmerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None): def _xmerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels):
tool, toolpath, binary, symlink, scriptfn = toolconf tool, toolpath, binary, symlink, scriptfn = toolconf
uipathfn = scmutil.getuipathfn(repo) uipathfn = scmutil.getuipathfn(repo)
if fcd.isabsent() or fco.isabsent(): if fcd.isabsent() or fco.isabsent():
repo.ui.warn( repo.ui.warn(
_(b'warning: %s cannot merge change/delete conflict for %s\n') _(b'warning: %s cannot merge change/delete conflict for %s\n')
% (tool, uipathfn(fcd.path())) % (tool, uipathfn(fcd.path()))
) )
return False, 1, None return False, 1, None