<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 20, 2016 at 5:47 AM, Pierre-Yves David <span dir="ltr"><<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank" class="cremed">pierre-yves.david@ens-lyon.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 12/20/2016 06:00 AM, Rodrigo Damazio wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Unfortunately, while set would match the right files, because of the way<br>
the code is structured, it provides no way to not try visiting the<br>
directories inside the non-recursive match - the set needs to first<br>
collect all the files in all subdirectories (match.py, _expandset) and<br>
then filter that down to the desired ones. In plain hg repos, that's<br>
just much slower - in the context of narrowhg, the repo will simply not<br>
have the manifests for those subdirectories and trying to visit them<br>
will crash.<br>
</blockquote>
<br></span>
Okay, so this seems like the current tools allow you to specify the right request but shortcoming of the -implementation- are preventing that request to work probably with narrowhg (and have performance impacts)<br>
<br>
Did I got that right ?</blockquote><div><br></div><div>Yes.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The follow-up change to this one (which I haven't sent yet but is<br>
written) is updating visitdir to allow non-recursiveness, which btw<br>
makes something like "hg files -I rootglob:browser/*" about 4-5x faster<br>
in the firefox repo.<br>
</blockquote>
<br></span>
And, If I read you right, the implementation of 'rootglob:' you provided in your patch have the same implementation issue, but you have another patch to improve the implementation to behave a way you can use (and is faster).<br>
<br>
Did I got that right too ?<br></blockquote><div><br></div><div>Yes.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">If I got these two pieces right, it looks like we could just apply the improvement to 'visitdir' to 'set:your/glob/*' and have your usecase filled while not jumping into UI changes. Would that work for you ?<br></blockquote><div><br></div><div>Not without a third set of changes, since set expansion doesn't use visitdir (or the matcher being built) at all - the dependency is that building the matcher depends on expanding the set (and thus the set can't depend on the matcher).</div><div>It would technically be doable for re:, but I'm wary of getting into the business of parsing and special-casing regexes to assume what they match or don't.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Fri, Dec 16, 2016 at 6:21 AM, Pierre-Yves David<br></span>
<<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank" class="cremed">pierre-yves.david@ens-lyon.or<wbr>g</a> <mailto:<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank" class="cremed">pierre-yves.david@ens-<wbr>lyon.org</a>>><span class=""><br>
wrote:<br>
<br>
<br>
<br>
On 12/16/2016 02:19 AM, Augie Fackler wrote:<br>
<br>
<br>
On Nov 24, 2016, at 10:28 AM, FUJIWARA Katsunori<br></span><div><div class="h5">
<<a href="mailto:foozy@lares.dti.ne.jp" target="_blank" class="cremed">foozy@lares.dti.ne.jp</a> <mailto:<a href="mailto:foozy@lares.dti.ne.jp" target="_blank" class="cremed">foozy@lares.dti.ne.jp</a>><wbr>> wrote:<br>
<br>
Yes, "files:" was the original version of this patch<br>
and the case I really<br>
care about :) I changed it to rootglob after your<br>
comments.<br>
Which way would be preferred to move forward?<br>
<br>
<br>
"files:" is "path:" family, and "rootglob:" is "glob:"<br>
family. As we<br>
concluded before, "path:" itself can't control recursion of<br>
matching<br>
well.<br>
<br>
Therefore, I think that "files:" should be implemented if<br>
needed,<br>
regardless of implementing "rootglob:".<br>
<br>
Of course, we need high point view of this area, at first :-)<br>
<br>
<br>
BTW, it is a little ambiguous (at least, for me) that<br>
"files:foo"<br>
matches against both file "foo" and files just under directory<br>
"foo". Name other than "files:" may resolve this ambiguity,<br>
but I<br>
don't have any better (and short enough) name :-<<br>
<br>
========== ==== ======= ===========<br>
pattern foo foo/bar foo/bar/baz<br>
========== ==== ======= ===========<br>
path:foo o o o<br>
<br>
files:foo o o x<br>
<br>
file:foo o x x<br>
dir:foo x o o<br>
========== ==== ======= ===========<br>
<br>
<br>
Scanning the plan page, I see that there’s a *lot* of work that<br>
could be done and no consensus as yet, but that the only<br>
immediate use case seems to be the rootfile/rootglob case. Is<br>
there some path forward we could agree on that would unblock<br>
those immediate needs for narrowhg and not make things harder in<br>
the future?<br>
<br>
Alternatively, would we be okay with a slight refactor of the<br>
matcher so that narrowhg can introduce a custom filesonly:<br>
matcher for the time being so we can keep making forward<br>
progress there? I don’t know the matcher code well enough to be<br>
able to guess if this is a reasonable path so we can be unblocked.<br>
<br>
(It’s very hard for to justify the amount of work implied by<br>
reaching consensus on FileNamePatternsPlan and then executing<br>
the entire thing when what we need is solvable today with a<br>
sub-hour patch to existing code, thus my trying to find a<br>
solution we can all live with.)<br>
<br>
<br>
As far as I understand, Foozy finding shows that the feature<br>
narrowhg needs is already there and nothing new is necessary.<br>
<br>
You can add "set:" in front of your glob to make it non recursive in<br>
all cases "set:your/directory/you/want/t<wbr>o/match/files/in/*"<br>
<br>
If this does not fits your needs, this probably mean I got your<br>
usecase wrong. In that case can you re-explain the issue you are<br>
trying to solve here?<br>
<br>
<br>
At the project level, it will make sense to clean up the Pattern<br>
Matching at some point, and Foozy wiki work will help us to do that.<br>
<br>
Cheers.<br>
<br>
--<br>
Pierre-Yves David<br>
<br>
<br>
</div></div></blockquote><span class="HOEnZb"><font color="#888888">
<br>
-- <br>
Pierre-Yves David<br>
</font></span></blockquote></div><br></div></div>