Page MenuHomePhabricator

filemerge: fix a missing attribute usage
ClosedPublic

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

Details

Summary

Flagged by both pytype and VSCode.

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:13 AM
dlax added inline comments.
mercurial/filemerge.py
122

What about fctx? It could also lack the ctx() method if an instance of absentfilectx.
I wonder if the intent was not fctx.changectx() == self.changectx().

mharbison72 added inline comments.Nov 21 2019, 7:37 AM
mercurial/filemerge.py
122

I’m not sure how this works, but this was enough to make pytype happy. Does it make any sense to compare an absent file to an absent file?

Maybe the right thing is to add a ctx attribute here?

dlax added inline comments.Nov 21 2019, 7:56 AM
mercurial/filemerge.py
122

Does it make any sense to compare an absent file to an absent file?

Not sure, but due to lazy evaluation fctx.ctx() == self.ctx() would only be evaluated if fctx is an absent file, hence also lacking the ctx() method.
I suggested to use changectx() because this is already implemented and also part of the basefilectx interface.

mharbison72 updated this revision to Diff 18415.Dec 1 2019, 4:58 PM
dlax accepted this revision.Dec 2 2019, 3:23 AM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.