( )⚙ D1747 repoview: add visibilityexception argument to filterrevs() and related fns

This is an archive of the discontinued Mercurial Phabricator instance.

repoview: add visibilityexception argument to filterrevs() and related fns
ClosedPublic

Authored by pulkit on Dec 22 2017, 12:56 PM.

Details

Summary

After this patch, filterrevs() can take an optional argument
visibilityexceptions which is a set of revs which should be exception to
being hidden. The visibilityexceptions will be passed to the function computing
hidden revisions for that filtername and are considered there while calculating
the set of hidden revs.

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

pulkit created this revision.Dec 22 2017, 12:56 PM
yuja requested changes to this revision.Dec 25 2017, 7:54 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/repoview.py
160

Since revisions are dynamically unhidden, we can't cache the result
to the (unfiltered) repo per filterename.

Perhaps we could apply visibilityexceptions at repoview.changelog().

revs = filterrevs(unfi, self.filtername)
revs = revs - self._visibilityexceptions
This revision now requires changes to proceed.Dec 25 2017, 7:54 AM
yuja added a comment.Dec 25 2017, 8:03 AM

Since revisions are dynamically unhidden, we can't cache the result
to the (unfiltered) repo per filterename.

FWIW, if the cache don't include "unhidden" revisions, maybe we don't
need new filter names like 'visible-hidden"?

In D1747#29916, @yuja wrote:

Since revisions are dynamically unhidden, we can't cache the result
to the (unfiltered) repo per filterename.

FWIW, if the cache don't include "unhidden" revisions, maybe we don't
need new filter names like 'visible-hidden"?

Yep I agree, but I wanted be on the safe side to prevent unknown bugs by using the same filtername.

mercurial/repoview.py
160

One reason to have this considered in computehidden() is we want this to be considered in _revealancestors() otherwise things which needs parent of hidden revision will throw an error like export. I will make it not cache the result if visibilityexceptions is present.

pulkit updated this revision to Diff 4607.Dec 25 2017, 10:08 AM
yuja accepted this revision.Dec 26 2017, 8:42 AM
This revision is now accepted and ready to land.Dec 26 2017, 8:42 AM
yuja added a comment.Dec 26 2017, 8:47 AM

FWIW, if the cache don't include "unhidden" revisions, maybe we don't
need new filter names like 'visible-hidden"?

Yep I agree, but I wanted be on the safe side to prevent unknown bugs by using the same filtername.

Actually we have to use new filtername to separate branch/tags caches until
we can disable these caches when revisions are dynamically pinned.

Queued the series, thanks.

This revision was automatically updated to reflect the committed changes.