D1527: context: add an abstract base class for filectx

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Fri Dec 1 19:44:09 UTC 2017


martinvonz added a comment.


  In https://phab.mercurial-scm.org/D1527#25742, @phillco wrote:
  
  > 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?

INLINE COMMENTS

> context.py:761
>      def _filelog(self):
>          return self._repo.file(self._path)
>  

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?

> context.py:2646
> +
> +    # TODO deduplicate these from ``basefilectx``
> +    def isbinary(self):

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

> context.py:2673
>      def decodeddata(self):
>          with open(self._path, "rb") as f:
>              return f.read()

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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1527

To: phillco, #hg-reviewers, martinvonz
Cc: mercurial-devel


More information about the Mercurial-devel mailing list