This is an archive of the discontinued Mercurial Phabricator instance.

match: write forceincludematcher using unionmatcher
ClosedPublic

Authored by martinvonz on Jul 11 2017, 8:01 PM.

Details

Summary

The forceincludematcher is simply a unionmatcher of a includematcher
(matching paths recursively) with the given matcher. Since the
forceincludematcher is only used by sparse, move it there.

I don't have a good sparse repo setup to test performance impact on.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Jul 11 2017, 8:01 PM
dsp added a subscriber: dsp.Jul 12 2017, 10:41 AM

I am slightly -1 on this, as it removes __repr__ for forceincludematcher. This would lead to remove information from hg debugwalk.

In D57#731, @dsp wrote:

I am slightly -1 on this, as it removes __repr__ for forceincludematcher. This would lead to remove information from hg debugwalk.

What arguments do you pass to "hg debugwalk" to get a forceincludematcher?

durham added a subscriber: durham.Jul 12 2017, 12:29 PM

makeittso

Might be nice to have unit tests for matchers, now that they're getting some complications.

In D57#743, @durham wrote:

makeittso
Might be nice to have unit tests for matchers, now that they're getting some complications.

I think the most interesting part to test would be visitdir(), because that's not tested much by our test suite ("def visitdir(self, dir): return True" would pass almost all tests). So it seems to me like D58 would benefit more from unit tests. patternmatcher and includematcher are more complex and would benefit more from unit tests.

dsp added a comment.Jul 12 2017, 7:28 PM
In D57#735, @martinvonz wrote:

What arguments do you pass to "hg debugwalk" to get a forceincludematcher?

You are right, as we don't use forceincludematcher inside match() it never gets into debugwalk. Your patch seems fine.

dsp accepted this revision.Jul 13 2017, 2:05 PM

accepting this for visibility to hg-reviewers.

This revision is now accepted and ready to land.Jul 13 2017, 2:05 PM
martinvonz updated this revision to Diff 128.Jul 14 2017, 2:47 AM
This revision now requires review to proceed.Jul 14 2017, 2:47 AM
This revision was automatically updated to reflect the committed changes.