Page MenuHomePhabricator

filemerge: fix a missing attribute usage
Needs ReviewPublic

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

Details

Reviewers
dlax
Group Reviewers
hg-reviewers
Summary

Flagged by both pytype and VSCode.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mharbison72 created this revision.Wed, Nov 20, 10:27 PM
dlax added a subscriber: dlax.Thu, Nov 21, 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.Thu, Nov 21, 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.Thu, Nov 21, 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.Sun, Dec 1, 4:58 PM
dlax accepted this revision.Mon, Dec 2, 3:23 AM