This is an archive of the discontinued Mercurial Phabricator instance.

context: add an abstract base class for filectx
AbandonedPublic

Authored by phillco on Nov 27 2017, 10:35 PM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Summary

basectx is very well and good, but we have two other filectx-like classes
that do not inherit from it (arbitraryfilectx and absentfilectx`), and
cannot, because basectx is still rather tied to representing a file in a
changectx.

Let's add some structure to these classes with a mininum set of abstract
methods enforced by the ABC module.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

phillco created this revision.Nov 27 2017, 10:35 PM

Note that we can't actually make absentfilectx based off this class today because of the context -> merge imprt cycle.

Note that we can't actually make absentfilectx based off this class today because of the context -> merge imprt cycle.

What exactly is that cycle? Can we break one of the links with a local import?

mercurial/context.py
761

For a separate patch: I find it weird to refer to these undefined fields (_repo and _path). Can we pass these into the constructor (which is currently not defined) and assign them there?

2625

Can we put them in abstractfilectx if they seems generic enough? We can still created optimized versions in subclasses.

2652

Maybe in a separate patch, you could make this just "return self.data()"?

phillco updated this revision to Diff 4175.Dec 7 2017, 4:22 PM

What exactly is that cycle? Can we break one of the links with a local import?

It's the same filectx -> ctx -> fileset -> merge -> filemerge cycle that frustrated earlier. We can't circumvent it this time (I think) because the import is in the module, not a function.

phillco planned changes to this revision.Dec 11 2017, 3:28 PM
phillco abandoned this revision.Jan 21 2018, 11:22 PM