This is an archive of the discontinued Mercurial Phabricator instance.

filemerge: drop a default argument to appease pytype
ClosedPublic

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

Details

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
Lint Skipped
Unit
Unit Tests Skipped

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)

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.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.