Page MenuHomePhabricator

repoview: define filteredchangelog as a top-level (non-local) class
ClosedPublic

Authored by martinvonz on Wed, Nov 6, 11:27 AM.

Details

Summary

As suggested by Greg. This makes it easier for extensions to override
the filtering.

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

martinvonz created this revision.Wed, Nov 6, 11:27 AM
indygreg accepted this revision.Thu, Nov 7, 3:32 AM

type() for the win! Thank you for this refactor!

This revision is now accepted and ready to land.Thu, Nov 7, 3:32 AM
yuja added a subscriber: yuja.Fri, Nov 8, 9:31 AM

+ cl.class = type('filteredchangelog',
+ (filteredchangelogmixin, cl.class),
+ {})

Nit: maybe this can be just class filteredchangelog(...): pass since
we don't name the type dynamically.

And we might need a class cache like _filteredrepotypes to work around
leaks of dynamically-created types.

In D7256#107172, @yuja wrote:

+ cl.class = type('filteredchangelog',
+ (filteredchangelogmixin, cl.class),
+ {})

Nit: maybe this can be just class filteredchangelog(...): pass since
we don't name the type dynamically.

Done in https://www.mercurial-scm.org/repo/hg/rev/85628a595c37.

And we might need a class cache like _filteredrepotypes to work around
leaks of dynamically-created types.

It seems like there's some caching in _clcache already. Maybe that's enough?

yuja added a comment.Thu, Nov 14, 8:51 AM
> And we might need a class cache like _filteredrepotypes to work around
> leaks of dynamically-created types.
It seems like there's some caching in `_clcache` already. Maybe that's enough?

Maybe not because clcache won't live longer than the repo instance, but
I have no idea if that actually impact the leak seen in hgweb.