Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

On Sun, Feb 5, 2017 at 4:01 AM, FUJIWARA Katsunori 
wrote:

> At Fri, 3 Feb 2017 20:26:04 -0800,
> Rodrigo Damazio wrote:
> >
> > [1  ]
> > [1.1  ]
> > Finally working on this again.
> > On point which I discussed with Martin offline - which feels more
> intuitive
> > as a prefix, "root" or "abs"? (so, "rootfilesin" or "absfilesin"?) We
> think
> > it's "abs", but wanted to make sure that's OK with others.
>
> In FileNamePatternsPlan page, I choose "root" as a name of point, to
> which patterns are relative ("root", "cwd", and "any").
>
> I'm OK with "abs", if other thinks that it is more intuitive for
> "relative to the root".
>

Alright, I'll keep "root", it sounds more consistent when put that way.
(Augie also seems to prefer root)

BTW, are "cwd" and "any" prefixes are OK with "abs" ?


> > Thanks
> >
> >
> > On Sun, Jan 29, 2017 at 3:15 AM, FUJIWARA Katsunori <
> fo...@lares.dti.ne.jp>
> > wrote:
> >
> > >
> > > At Fri, 27 Jan 2017 15:14:38 -0800,
> > > Rodrigo Damazio wrote:
> > > >
> > > > [1  ]
> > > > [1.1  ]
> > > > On Fri, Jan 27, 2017 at 1:03 AM, FUJIWARA Katsunori <
> > > fo...@lares.dti.ne.jp>
> > > > wrote:
> > > >
> > > > >
> > > > > At Thu, 26 Jan 2017 17:27:17 -0800,
> > > > > Rodrigo Damazio wrote:
> > > > > >
> > > > > > [1  ]
> > > > > > [1.1  ]
> > > > > > All sounds very reasonable, and "filesin:" or "rootfilesin:"
> LGTM.
> > > > >
> > > > > Is it OK for your solution that "rootfilesin:FOO" doesn't match
> > > > > against "file FOO", even though your patch posted in this thread
> made
> > > > > "files:FOO" do so ? or, is combining "rootfile:" and "rootfilesin"
> > > > > acceptable for your solution ?
> > > > >
> > > >
> > > > Yes, not matching files is fine, and actually the easiest to
> implement
> > > (the
> > > > regex is simpler and our custom server doesn't support files anyway).
> > > > For that, rootfilesin:foo/bar can produce regex ^foo/bar/[^/]+$ or
> > > similar
> > > > which would not match a file called bar. visitdir would have to be
> > > updated
> > > > accordingly, of course, but that shouldn't be too hard (and i can
> take
> > > the
> > > > opportunity to add some comments to the code).
> > > >
> > > > If that looks good to you, let me know and I'll send an updated
> patch.
> > >
> > > Sure, LGTM
> > >
> > > --
> > > --
> > > [FUJIWARA Katsunori] fo...@lares.dti.ne.jp
> > >
> > [1.2  ]
> >
> > [2 S/MIME Cryptographic Signature ]
> >
>
> --
> --
> [FUJIWARA Katsunori] fo...@lares.dti.ne.jp
>


smime.p7s
Description: S/MIME Cryptographic Signature
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[FUJIWARA Katsunori]

At Fri, 3 Feb 2017 20:26:04 -0800,
Rodrigo Damazio wrote:
> 
> [1  ]
> [1.1  ]
> Finally working on this again.
> On point which I discussed with Martin offline - which feels more intuitive
> as a prefix, "root" or "abs"? (so, "rootfilesin" or "absfilesin"?) We think
> it's "abs", but wanted to make sure that's OK with others.

In FileNamePatternsPlan page, I choose "root" as a name of point, to
which patterns are relative ("root", "cwd", and "any").

I'm OK with "abs", if other thinks that it is more intuitive for
"relative to the root".

BTW, are "cwd" and "any" prefixes are OK with "abs" ?

> Thanks
> 
> 
> On Sun, Jan 29, 2017 at 3:15 AM, FUJIWARA Katsunori 
> wrote:
> 
> >
> > At Fri, 27 Jan 2017 15:14:38 -0800,
> > Rodrigo Damazio wrote:
> > >
> > > [1  ]
> > > [1.1  ]
> > > On Fri, Jan 27, 2017 at 1:03 AM, FUJIWARA Katsunori <
> > fo...@lares.dti.ne.jp>
> > > wrote:
> > >
> > > >
> > > > At Thu, 26 Jan 2017 17:27:17 -0800,
> > > > Rodrigo Damazio wrote:
> > > > >
> > > > > [1  ]
> > > > > [1.1  ]
> > > > > All sounds very reasonable, and "filesin:" or "rootfilesin:" LGTM.
> > > >
> > > > Is it OK for your solution that "rootfilesin:FOO" doesn't match
> > > > against "file FOO", even though your patch posted in this thread made
> > > > "files:FOO" do so ? or, is combining "rootfile:" and "rootfilesin"
> > > > acceptable for your solution ?
> > > >
> > >
> > > Yes, not matching files is fine, and actually the easiest to implement
> > (the
> > > regex is simpler and our custom server doesn't support files anyway).
> > > For that, rootfilesin:foo/bar can produce regex ^foo/bar/[^/]+$ or
> > similar
> > > which would not match a file called bar. visitdir would have to be
> > updated
> > > accordingly, of course, but that shouldn't be too hard (and i can take
> > the
> > > opportunity to add some comments to the code).
> > >
> > > If that looks good to you, let me know and I'll send an updated patch.
> >
> > Sure, LGTM
> >
> > --
> > --
> > [FUJIWARA Katsunori] fo...@lares.dti.ne.jp
> >
> [1.2  ]
> 
> [2 S/MIME Cryptographic Signature ]
> 

-- 
--
[FUJIWARA Katsunori] fo...@lares.dti.ne.jp
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

Finally working on this again.
On point which I discussed with Martin offline - which feels more intuitive
as a prefix, "root" or "abs"? (so, "rootfilesin" or "absfilesin"?) We think
it's "abs", but wanted to make sure that's OK with others.

Thanks


On Sun, Jan 29, 2017 at 3:15 AM, FUJIWARA Katsunori 
wrote:

>
> At Fri, 27 Jan 2017 15:14:38 -0800,
> Rodrigo Damazio wrote:
> >
> > [1  ]
> > [1.1  ]
> > On Fri, Jan 27, 2017 at 1:03 AM, FUJIWARA Katsunori <
> fo...@lares.dti.ne.jp>
> > wrote:
> >
> > >
> > > At Thu, 26 Jan 2017 17:27:17 -0800,
> > > Rodrigo Damazio wrote:
> > > >
> > > > [1  ]
> > > > [1.1  ]
> > > > All sounds very reasonable, and "filesin:" or "rootfilesin:" LGTM.
> > >
> > > Is it OK for your solution that "rootfilesin:FOO" doesn't match
> > > against "file FOO", even though your patch posted in this thread made
> > > "files:FOO" do so ? or, is combining "rootfile:" and "rootfilesin"
> > > acceptable for your solution ?
> > >
> >
> > Yes, not matching files is fine, and actually the easiest to implement
> (the
> > regex is simpler and our custom server doesn't support files anyway).
> > For that, rootfilesin:foo/bar can produce regex ^foo/bar/[^/]+$ or
> similar
> > which would not match a file called bar. visitdir would have to be
> updated
> > accordingly, of course, but that shouldn't be too hard (and i can take
> the
> > opportunity to add some comments to the code).
> >
> > If that looks good to you, let me know and I'll send an updated patch.
>
> Sure, LGTM
>
> --
> --
> [FUJIWARA Katsunori] fo...@lares.dti.ne.jp
>


smime.p7s
Description: S/MIME Cryptographic Signature
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[FUJIWARA Katsunori]

At Fri, 27 Jan 2017 15:14:38 -0800,
Rodrigo Damazio wrote:
> 
> [1  ]
> [1.1  ]
> On Fri, Jan 27, 2017 at 1:03 AM, FUJIWARA Katsunori 
> wrote:
> 
> >
> > At Thu, 26 Jan 2017 17:27:17 -0800,
> > Rodrigo Damazio wrote:
> > >
> > > [1  ]
> > > [1.1  ]
> > > All sounds very reasonable, and "filesin:" or "rootfilesin:" LGTM.
> >
> > Is it OK for your solution that "rootfilesin:FOO" doesn't match
> > against "file FOO", even though your patch posted in this thread made
> > "files:FOO" do so ? or, is combining "rootfile:" and "rootfilesin"
> > acceptable for your solution ?
> >
> 
> Yes, not matching files is fine, and actually the easiest to implement (the
> regex is simpler and our custom server doesn't support files anyway).
> For that, rootfilesin:foo/bar can produce regex ^foo/bar/[^/]+$ or similar
> which would not match a file called bar. visitdir would have to be updated
> accordingly, of course, but that shouldn't be too hard (and i can take the
> opportunity to add some comments to the code).
> 
> If that looks good to you, let me know and I'll send an updated patch.

Sure, LGTM

-- 
--
[FUJIWARA Katsunori] fo...@lares.dti.ne.jp
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

On Fri, Jan 27, 2017 at 1:03 AM, FUJIWARA Katsunori 
wrote:

>
> At Thu, 26 Jan 2017 17:27:17 -0800,
> Rodrigo Damazio wrote:
> >
> > [1  ]
> > [1.1  ]
> > All sounds very reasonable, and "filesin:" or "rootfilesin:" LGTM.
>
> Is it OK for your solution that "rootfilesin:FOO" doesn't match
> against "file FOO", even though your patch posted in this thread made
> "files:FOO" do so ? or, is combining "rootfile:" and "rootfilesin"
> acceptable for your solution ?
>

Yes, not matching files is fine, and actually the easiest to implement (the
regex is simpler and our custom server doesn't support files anyway).
For that, rootfilesin:foo/bar can produce regex ^foo/bar/[^/]+$ or similar
which would not match a file called bar. visitdir would have to be updated
accordingly, of course, but that shouldn't be too hard (and i can take the
opportunity to add some comments to the code).

If that looks good to you, let me know and I'll send an updated patch.

> On Thu, Jan 26, 2017 at 11:24 AM, Martin von Zweigbergk <
> > martinv...@google.com> wrote:
> >
> > > On Thu, Jan 26, 2017 at 11:19 AM, FUJIWARA Katsunori
> > >  wrote:
> > > >
> > > > At Wed, 25 Jan 2017 20:54:37 -0800,
> > > > Martin von Zweigbergk wrote:
> > > >>
> > > >> On Mon, Jan 23, 2017 at 5:02 PM, Rodrigo Damazio via Mercurial-devel
> > > >>  wrote:
> > > >> > Getting back to this after the end-of-year hiatus (yes, I know it
> > > happens to
> > > >> > be during another code freeze :) I seem to have good timing).
> > > >> >
> > > >> > On Tue, Dec 27, 2016 at 2:14 AM, Pierre-Yves David
> > > >> >  wrote:
> > > >> >>
> > > >> >>
> > > >> >>
> > > >> >> On 12/21/2016 04:21 AM, Rodrigo Damazio wrote:
> > > >> >>>
> > > >> >>> 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 ?
> > > >> >>>
> > > >> >>>
> > > >> >>> 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).
> > > >> >>> 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.
> > > >> >>
> > > >> >>
> > > >> >> Rodrigo and I chatted directly about this a couple of days ago.
> Here
> > > is a
> > > >> >> quick summary of my new understanding of the situation.
> > > >> >>
> > > >> >> Fileset
> > > >> >> ---
> > > >> >>
> > > >> >> Fileset (behind "set:") can give the right result, but it is
> powered
> > > by
> > > >> >> not very modern code, it follow the old revset principle of "get
> > > everything
> > > >> >> and then run filters on that everything". That does not fit
> Rodrigo
> > > needs at
> > > >> >> all. It was easy to make 'set:' a bit smarter in the simple case
> but
> > > then we
> > > >> >> get into the issue that the matcher class is using 'set:' in a
> > > strange,
> > > >> >> non-lazy, way that does not use all the 'visitdir' feature
> > > Rodrigo/Google
> > > >> >> needs.
> > > >> >>
> > > >> >> So in short, fileset needs a rework before being usable in a
> > > demanding
> > > >> >> context.
> > > >> >>
> > > >> >>
> > > >> >> Current path restriction capability
> > > >> >> ---
> > > >> >>
> > > >> >> The 'Match' class already have logic to restrict the path visited
> > > >> >> (implemented in the 'visitdir' method). To clarify, this logic
> as no
> > > effect
> > > >> >> on the returned match but is only an optimization for the
> directory
> > > we
> > > >> >> visit. It seems to only kicks in when treemanifest is used.
> > > >> >> This logic already works with a couple of patterns type (all
> pattern
> > > use
> > > >> >> the same class). However, that logic currently do not support the
> > > case were
> > > >> >> one want to select some subdirectory and skips the rest of the
> > > subtrees
> > > >> >> under it.
> > > >> >
> > > >> >
> > > >> > That is correct.
> > > >> >
> > > >> >> note: Rodrigo, you seems to have a good understanding of the
> logic.
> > > Do you
> > > >> >> think you could document the involved attributes (_includeroots,
> > > >> >> _includedirs, _excluderoots, etc) That would help a lot the next
> > > poor souls
> > > >> >> looking at the code.
> > > >> >
> > > >> >
> > > >> > Sure. It took me a while to understand that "roots" means
> "recursive
> > > >> > directories" and "dirs" means "non-recursive directories" in that
> > > code - it
> > > >> > all became much more clear after that. I'll be sure to add
> comments
> > > in my
> 

Re: [PATCH] match: adding non-recursive directory matching

[FUJIWARA Katsunori]

At Thu, 26 Jan 2017 17:27:17 -0800,
Rodrigo Damazio wrote:
> 
> [1  ]
> [1.1  ]
> All sounds very reasonable, and "filesin:" or "rootfilesin:" LGTM.

Is it OK for your solution that "rootfilesin:FOO" doesn't match
against "file FOO", even though your patch posted in this thread made
"files:FOO" do so ? or, is combining "rootfile:" and "rootfilesin"
acceptable for your solution ?


> On Thu, Jan 26, 2017 at 11:24 AM, Martin von Zweigbergk <
> martinv...@google.com> wrote:
> 
> > On Thu, Jan 26, 2017 at 11:19 AM, FUJIWARA Katsunori
> >  wrote:
> > >
> > > At Wed, 25 Jan 2017 20:54:37 -0800,
> > > Martin von Zweigbergk wrote:
> > >>
> > >> On Mon, Jan 23, 2017 at 5:02 PM, Rodrigo Damazio via Mercurial-devel
> > >>  wrote:
> > >> > Getting back to this after the end-of-year hiatus (yes, I know it
> > happens to
> > >> > be during another code freeze :) I seem to have good timing).
> > >> >
> > >> > On Tue, Dec 27, 2016 at 2:14 AM, Pierre-Yves David
> > >> >  wrote:
> > >> >>
> > >> >>
> > >> >>
> > >> >> On 12/21/2016 04:21 AM, Rodrigo Damazio wrote:
> > >> >>>
> > >> >>> 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 ?
> > >> >>>
> > >> >>>
> > >> >>> 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).
> > >> >>> 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.
> > >> >>
> > >> >>
> > >> >> Rodrigo and I chatted directly about this a couple of days ago. Here
> > is a
> > >> >> quick summary of my new understanding of the situation.
> > >> >>
> > >> >> Fileset
> > >> >> ---
> > >> >>
> > >> >> Fileset (behind "set:") can give the right result, but it is powered
> > by
> > >> >> not very modern code, it follow the old revset principle of "get
> > everything
> > >> >> and then run filters on that everything". That does not fit Rodrigo
> > needs at
> > >> >> all. It was easy to make 'set:' a bit smarter in the simple case but
> > then we
> > >> >> get into the issue that the matcher class is using 'set:' in a
> > strange,
> > >> >> non-lazy, way that does not use all the 'visitdir' feature
> > Rodrigo/Google
> > >> >> needs.
> > >> >>
> > >> >> So in short, fileset needs a rework before being usable in a
> > demanding
> > >> >> context.
> > >> >>
> > >> >>
> > >> >> Current path restriction capability
> > >> >> ---
> > >> >>
> > >> >> The 'Match' class already have logic to restrict the path visited
> > >> >> (implemented in the 'visitdir' method). To clarify, this logic as no
> > effect
> > >> >> on the returned match but is only an optimization for the directory
> > we
> > >> >> visit. It seems to only kicks in when treemanifest is used.
> > >> >> This logic already works with a couple of patterns type (all pattern
> > use
> > >> >> the same class). However, that logic currently do not support the
> > case were
> > >> >> one want to select some subdirectory and skips the rest of the
> > subtrees
> > >> >> under it.
> > >> >
> > >> >
> > >> > That is correct.
> > >> >
> > >> >> note: Rodrigo, you seems to have a good understanding of the logic.
> > Do you
> > >> >> think you could document the involved attributes (_includeroots,
> > >> >> _includedirs, _excluderoots, etc) That would help a lot the next
> > poor souls
> > >> >> looking at the code.
> > >> >
> > >> >
> > >> > Sure. It took me a while to understand that "roots" means "recursive
> > >> > directories" and "dirs" means "non-recursive directories" in that
> > code - it
> > >> > all became much more clear after that. I'll be sure to add comments
> > in my
> > >> > patch and/or rename the attributes.
> > >> >
> > >> >>
> > >> >>
> > >> >> Way forward
> > >> >> ---
> > >> >>
> > >> >> That limitation in the matcher class optimization is the main
> > blocker for
> > >> >> Rodrigo/Google needs. The optimization is independent of the UI part
> > we
> > >> >> actually provides to user as all patterns use the same matcher class
> > and
> > >> >> some existing class could already benefit from this optimization.
> > >> >>
> > >> >> Rodrigo seems to have a patch to update the matcher code to track and
> > >> >> optimize the "subdir-but-not-subtree" case. He has not submitted
> > this patch
> > >> >> yet. Submitting that patches seems the next step to me. It will get
> > the
> > >> >> matcher code in a state that can actually be used for the
> > >> >> 

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

All sounds very reasonable, and "filesin:" or "rootfilesin:" LGTM.


On Thu, Jan 26, 2017 at 11:24 AM, Martin von Zweigbergk <
martinv...@google.com> wrote:

> On Thu, Jan 26, 2017 at 11:19 AM, FUJIWARA Katsunori
>  wrote:
> >
> > At Wed, 25 Jan 2017 20:54:37 -0800,
> > Martin von Zweigbergk wrote:
> >>
> >> On Mon, Jan 23, 2017 at 5:02 PM, Rodrigo Damazio via Mercurial-devel
> >>  wrote:
> >> > Getting back to this after the end-of-year hiatus (yes, I know it
> happens to
> >> > be during another code freeze :) I seem to have good timing).
> >> >
> >> > On Tue, Dec 27, 2016 at 2:14 AM, Pierre-Yves David
> >> >  wrote:
> >> >>
> >> >>
> >> >>
> >> >> On 12/21/2016 04:21 AM, Rodrigo Damazio wrote:
> >> >>>
> >> >>> 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 ?
> >> >>>
> >> >>>
> >> >>> 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).
> >> >>> 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.
> >> >>
> >> >>
> >> >> Rodrigo and I chatted directly about this a couple of days ago. Here
> is a
> >> >> quick summary of my new understanding of the situation.
> >> >>
> >> >> Fileset
> >> >> ---
> >> >>
> >> >> Fileset (behind "set:") can give the right result, but it is powered
> by
> >> >> not very modern code, it follow the old revset principle of "get
> everything
> >> >> and then run filters on that everything". That does not fit Rodrigo
> needs at
> >> >> all. It was easy to make 'set:' a bit smarter in the simple case but
> then we
> >> >> get into the issue that the matcher class is using 'set:' in a
> strange,
> >> >> non-lazy, way that does not use all the 'visitdir' feature
> Rodrigo/Google
> >> >> needs.
> >> >>
> >> >> So in short, fileset needs a rework before being usable in a
> demanding
> >> >> context.
> >> >>
> >> >>
> >> >> Current path restriction capability
> >> >> ---
> >> >>
> >> >> The 'Match' class already have logic to restrict the path visited
> >> >> (implemented in the 'visitdir' method). To clarify, this logic as no
> effect
> >> >> on the returned match but is only an optimization for the directory
> we
> >> >> visit. It seems to only kicks in when treemanifest is used.
> >> >> This logic already works with a couple of patterns type (all pattern
> use
> >> >> the same class). However, that logic currently do not support the
> case were
> >> >> one want to select some subdirectory and skips the rest of the
> subtrees
> >> >> under it.
> >> >
> >> >
> >> > That is correct.
> >> >
> >> >> note: Rodrigo, you seems to have a good understanding of the logic.
> Do you
> >> >> think you could document the involved attributes (_includeroots,
> >> >> _includedirs, _excluderoots, etc) That would help a lot the next
> poor souls
> >> >> looking at the code.
> >> >
> >> >
> >> > Sure. It took me a while to understand that "roots" means "recursive
> >> > directories" and "dirs" means "non-recursive directories" in that
> code - it
> >> > all became much more clear after that. I'll be sure to add comments
> in my
> >> > patch and/or rename the attributes.
> >> >
> >> >>
> >> >>
> >> >> Way forward
> >> >> ---
> >> >>
> >> >> That limitation in the matcher class optimization is the main
> blocker for
> >> >> Rodrigo/Google needs. The optimization is independent of the UI part
> we
> >> >> actually provides to user as all patterns use the same matcher class
> and
> >> >> some existing class could already benefit from this optimization.
> >> >>
> >> >> Rodrigo seems to have a patch to update the matcher code to track and
> >> >> optimize the "subdir-but-not-subtree" case. He has not submitted
> this patch
> >> >> yet. Submitting that patches seems the next step to me. It will get
> the
> >> >> matcher code in a state that can actually be used for the
> >> >> narrowhg+treemanifest usecase.
> >> >>
> >> >> Once that code is in, it seems easy to make sure various patterns
> use it
> >> >> basic, easily recognizable cases. We poked at updating the code to
> have
> >> >> basic regexp matching a subtree recognized as such and that was
> quite easy.
> >> >>
> >> >>
> >> >> Rodrigo, does that match your current understanding of the situation?
> >> >
> >> >
> >> > It does.
> >> > And just to clarify on the patches - I sent an initial patch, then
> after
> >> > comments changes it significantly, so those are two different changes:
> >> 

Re: [PATCH] match: adding non-recursive directory matching

[Martin von Zweigbergk via Mercurial-devel]

On Thu, Jan 26, 2017 at 11:19 AM, FUJIWARA Katsunori
 wrote:
>
> At Wed, 25 Jan 2017 20:54:37 -0800,
> Martin von Zweigbergk wrote:
>>
>> On Mon, Jan 23, 2017 at 5:02 PM, Rodrigo Damazio via Mercurial-devel
>>  wrote:
>> > Getting back to this after the end-of-year hiatus (yes, I know it happens 
>> > to
>> > be during another code freeze :) I seem to have good timing).
>> >
>> > On Tue, Dec 27, 2016 at 2:14 AM, Pierre-Yves David
>> >  wrote:
>> >>
>> >>
>> >>
>> >> On 12/21/2016 04:21 AM, Rodrigo Damazio wrote:
>> >>>
>> >>> 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 ?
>> >>>
>> >>>
>> >>> 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).
>> >>> 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.
>> >>
>> >>
>> >> Rodrigo and I chatted directly about this a couple of days ago. Here is a
>> >> quick summary of my new understanding of the situation.
>> >>
>> >> Fileset
>> >> ---
>> >>
>> >> Fileset (behind "set:") can give the right result, but it is powered by
>> >> not very modern code, it follow the old revset principle of "get 
>> >> everything
>> >> and then run filters on that everything". That does not fit Rodrigo needs 
>> >> at
>> >> all. It was easy to make 'set:' a bit smarter in the simple case but then 
>> >> we
>> >> get into the issue that the matcher class is using 'set:' in a strange,
>> >> non-lazy, way that does not use all the 'visitdir' feature Rodrigo/Google
>> >> needs.
>> >>
>> >> So in short, fileset needs a rework before being usable in a demanding
>> >> context.
>> >>
>> >>
>> >> Current path restriction capability
>> >> ---
>> >>
>> >> The 'Match' class already have logic to restrict the path visited
>> >> (implemented in the 'visitdir' method). To clarify, this logic as no 
>> >> effect
>> >> on the returned match but is only an optimization for the directory we
>> >> visit. It seems to only kicks in when treemanifest is used.
>> >> This logic already works with a couple of patterns type (all pattern use
>> >> the same class). However, that logic currently do not support the case 
>> >> were
>> >> one want to select some subdirectory and skips the rest of the subtrees
>> >> under it.
>> >
>> >
>> > That is correct.
>> >
>> >> note: Rodrigo, you seems to have a good understanding of the logic. Do you
>> >> think you could document the involved attributes (_includeroots,
>> >> _includedirs, _excluderoots, etc) That would help a lot the next poor 
>> >> souls
>> >> looking at the code.
>> >
>> >
>> > Sure. It took me a while to understand that "roots" means "recursive
>> > directories" and "dirs" means "non-recursive directories" in that code - it
>> > all became much more clear after that. I'll be sure to add comments in my
>> > patch and/or rename the attributes.
>> >
>> >>
>> >>
>> >> Way forward
>> >> ---
>> >>
>> >> That limitation in the matcher class optimization is the main blocker for
>> >> Rodrigo/Google needs. The optimization is independent of the UI part we
>> >> actually provides to user as all patterns use the same matcher class and
>> >> some existing class could already benefit from this optimization.
>> >>
>> >> Rodrigo seems to have a patch to update the matcher code to track and
>> >> optimize the "subdir-but-not-subtree" case. He has not submitted this 
>> >> patch
>> >> yet. Submitting that patches seems the next step to me. It will get the
>> >> matcher code in a state that can actually be used for the
>> >> narrowhg+treemanifest usecase.
>> >>
>> >> Once that code is in, it seems easy to make sure various patterns use it
>> >> basic, easily recognizable cases. We poked at updating the code to have
>> >> basic regexp matching a subtree recognized as such and that was quite 
>> >> easy.
>> >>
>> >>
>> >> Rodrigo, does that match your current understanding of the situation?
>> >
>> >
>> > It does.
>> > And just to clarify on the patches - I sent an initial patch, then after
>> > comments changes it significantly, so those are two different changes:
>> >
>> > The first implements a "files:" matcher which matches all files inside a
>> > directory, non-recursively. This has no wildcards, so special-casing it in
>> > visitdir and any other places needed results in clean and simple code ("if
>> > it's files:, don't recurse").
>> > The second implements "rootglob:" which allows any number of wildcards at
>> > point in 

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

Getting back to this after the end-of-year hiatus (yes, I know it happens
to be during another code freeze :) I seem to have good timing).

On Tue, Dec 27, 2016 at 2:14 AM, Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

>
>
> On 12/21/2016 04:21 AM, Rodrigo Damazio wrote:
>
>> 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 ?
>>
>>
>> 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).
>> 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.
>>
>
> Rodrigo and I chatted directly about this a couple of days ago. Here is a
> quick summary of my new understanding of the situation.
>
> Fileset
> ---
>
> Fileset (behind "set:") can give the right result, but it is powered by
> not very modern code, it follow the old revset principle of "get everything
> and then run filters on that everything". That does not fit Rodrigo needs
> at all. It was easy to make 'set:' a bit smarter in the simple case but
> then we get into the issue that the matcher class is using 'set:' in a
> strange, non-lazy, way that does not use all the 'visitdir' feature
> Rodrigo/Google needs.
>
> So in short, fileset needs a rework before being usable in a demanding
> context.
>
>
> Current path restriction capability
> ---
>
> The 'Match' class already have logic to restrict the path visited
> (implemented in the 'visitdir' method). To clarify, this logic as no effect
> on the returned match but is only an optimization for the directory we
> visit. It seems to only kicks in when treemanifest is used.
> This logic already works with a couple of patterns type (all pattern use
> the same class). However, that logic currently do not support the case were
> one want to select some subdirectory and skips the rest of the subtrees
> under it.
>

That is correct.

note: Rodrigo, you seems to have a good understanding of the logic. Do you
> think you could document the involved attributes (_includeroots,
> _includedirs, _excluderoots, etc) That would help a lot the next poor souls
> looking at the code.
>

Sure. It took me a while to understand that "roots" means "recursive
directories" and "dirs" means "non-recursive directories" in that code - it
all became much more clear after that. I'll be sure to add comments in my
patch and/or rename the attributes.


>
> Way forward
> ---
>
> That limitation in the matcher class optimization is the main blocker for
> Rodrigo/Google needs. The optimization is independent of the UI part we
> actually provides to user as all patterns use the same matcher class and
> some existing class could already benefit from this optimization.
>
> Rodrigo seems to have a patch to update the matcher code to track and
> optimize the "subdir-but-not-subtree" case. He has not submitted this patch
> yet. Submitting that patches seems the next step to me. It will get the
> matcher code in a state that can actually be used for the
> narrowhg+treemanifest usecase.
>
> Once that code is in, it seems easy to make sure various patterns use it
> basic, easily recognizable cases. We poked at updating the code to have
> basic regexp matching a subtree recognized as such and that was quite easy.
>
>
> Rodrigo, does that match your current understanding of the situation?
>

It does.
And just to clarify on the patches - I sent an initial patch, then after
comments changes it significantly, so those are two different changes:

   - The first implements a "files:" matcher which matches all files inside
   a directory, non-recursively. This has no wildcards, so special-casing it
   in visitdir and any other places needed results in clean and simple code
   ("if it's files:, don't recurse").
   - The second implements "rootglob:" which allows any number of wildcards
   at point in the path, and is part of Foozy's plan for the new set of
   matchers. This adds some complexity in splitting dirs and roots (mentioned
   above) by having to parse the wildcards, and then the visitdir change looks
   less clean ("if it's a rootglob that has a single /* wildcard at the end,
   then don't recurse" - other cases are possible but start to get more
   complex).

For these reasons, I'd still prefer to get "files:" or similar in, but I'm
open for doing it either way. Please advise on the preferred way and I'll
send an updated patch (2 patches really - one for the matcher, one for the
visitdir optimization which makes it work with narrow).

Thanks
Rodrigo


smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH] match: adding non-recursive directory matching

[Pierre-Yves David]

On 12/21/2016 04:21 AM, Rodrigo Damazio wrote:

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 ?


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).
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.


Rodrigo and I chatted directly about this a couple of days ago. Here is 
a quick summary of my new understanding of the situation.


Fileset
---

Fileset (behind "set:") can give the right result, but it is powered by 
not very modern code, it follow the old revset principle of "get 
everything and then run filters on that everything". That does not fit 
Rodrigo needs at all. It was easy to make 'set:' a bit smarter in the 
simple case but then we get into the issue that the matcher class is 
using 'set:' in a strange, non-lazy, way that does not use all the 
'visitdir' feature Rodrigo/Google needs.


So in short, fileset needs a rework before being usable in a demanding 
context.



Current path restriction capability
---

The 'Match' class already have logic to restrict the path visited 
(implemented in the 'visitdir' method). To clarify, this logic as no 
effect on the returned match but is only an optimization for the 
directory we visit. It seems to only kicks in when treemanifest is used.
This logic already works with a couple of patterns type (all pattern use 
the same class). However, that logic currently do not support the case 
were one want to select some subdirectory and skips the rest of the 
subtrees under it.


note: Rodrigo, you seems to have a good understanding of the logic. Do 
you think you could document the involved attributes (_includeroots, 
_includedirs, _excluderoots, etc) That would help a lot the next poor 
souls looking at the code.


Way forward
---

That limitation in the matcher class optimization is the main blocker 
for Rodrigo/Google needs. The optimization is independent of the UI part 
we actually provides to user as all patterns use the same matcher class 
and some existing class could already benefit from this optimization.


Rodrigo seems to have a patch to update the matcher code to track and 
optimize the "subdir-but-not-subtree" case. He has not submitted this 
patch yet. Submitting that patches seems the next step to me. It will 
get the matcher code in a state that can actually be used for the 
narrowhg+treemanifest usecase.


Once that code is in, it seems easy to make sure various patterns use it 
basic, easily recognizable cases. We poked at updating the code to have 
basic regexp matching a subtree recognized as such and that was quite easy.



Rodrigo, does that match your current understanding of the situation?

Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

On Tue, Dec 20, 2016 at 5:47 AM, Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

>
>
> On 12/20/2016 06:00 AM, Rodrigo Damazio wrote:
>
>> Unfortunately, while set would match the right files, because of the way
>> the code is structured, it provides no way to not try visiting the
>> directories inside the non-recursive match - the set needs to first
>> collect all the files in all subdirectories (match.py, _expandset) and
>> then filter that down to the desired ones. In plain hg repos, that's
>> just much slower - in the context of narrowhg, the repo will simply not
>> have the manifests for those subdirectories and trying to visit them
>> will crash.
>>
>
> 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)
>
> Did I got that right ?


Yes.

The follow-up change to this one (which I haven't sent yet but is
>> written) is updating visitdir to allow non-recursiveness, which btw
>> makes something like "hg files -I rootglob:browser/*" about 4-5x faster
>> in the firefox repo.
>>
>
> 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).
>
> Did I got that right too ?
>

Yes.

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 ?
>

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).
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.


> On Fri, Dec 16, 2016 at 6:21 AM, Pierre-Yves David
>> >
>> wrote:
>>
>>
>>
>> On 12/16/2016 02:19 AM, Augie Fackler wrote:
>>
>>
>> On Nov 24, 2016, at 10:28 AM, FUJIWARA Katsunori
>> > wrote:
>>
>> Yes, "files:" was the original version of this patch
>> and the case I really
>> care about :) I changed it to rootglob after your
>> comments.
>> Which way would be preferred to move forward?
>>
>>
>> "files:" is "path:" family, and "rootglob:" is "glob:"
>> family. As we
>> concluded before, "path:" itself can't control recursion of
>> matching
>> well.
>>
>> Therefore, I think that "files:" should be implemented if
>> needed,
>> regardless of implementing "rootglob:".
>>
>> Of course, we need high point view of this area, at first :-)
>>
>>
>> BTW, it is a little ambiguous (at least, for me) that
>> "files:foo"
>> matches against both file "foo" and files just under directory
>> "foo". Name other than "files:" may resolve this ambiguity,
>> but I
>> don't have any better (and short enough) name :-<
>>
>>  ==  === ===
>>  patternfoo  foo/bar foo/bar/baz
>>  ==  === ===
>>  path:fooo o o
>>
>>  files:foo   o o x
>>
>>  file:fooo x x
>>  dir:foo x o o
>>  ==  === ===
>>
>>
>> Scanning the plan page, I see that there’s a *lot* of work that
>> could be done and no consensus as yet, but that the only
>> immediate use case seems to be the rootfile/rootglob case. Is
>> there some path forward we could agree on that would unblock
>> those immediate needs for narrowhg and not make things harder in
>> the future?
>>
>> Alternatively, would we be okay with a slight refactor of the
>> matcher so that narrowhg can introduce a custom filesonly:
>> matcher for the time being so we can keep making forward
>> progress there?  I don’t know the matcher code well enough to be
>> able to guess if this is a reasonable path so we can be unblocked.
>>
>> (It’s very hard for to justify the amount of work implied by
>> reaching consensus on FileNamePatternsPlan and then executing
>> the entire thing when what we need is solvable today with a
>> sub-hour patch to existing code, thus my trying to find a
>> solution we can all live with.)
>>
>>

Re: [PATCH] match: adding non-recursive directory matching

[Pierre-Yves David]

On 12/20/2016 06:00 AM, Rodrigo Damazio wrote:

Unfortunately, while set would match the right files, because of the way
the code is structured, it provides no way to not try visiting the
directories inside the non-recursive match - the set needs to first
collect all the files in all subdirectories (match.py, _expandset) and
then filter that down to the desired ones. In plain hg repos, that's
just much slower - in the context of narrowhg, the repo will simply not
have the manifests for those subdirectories and trying to visit them
will crash.


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)


Did I got that right ?


The follow-up change to this one (which I haven't sent yet but is
written) is updating visitdir to allow non-recursiveness, which btw
makes something like "hg files -I rootglob:browser/*" about 4-5x faster
in the firefox repo.


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).


Did I got that right too ?


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 ?



On Fri, Dec 16, 2016 at 6:21 AM, Pierre-Yves David
>
wrote:



On 12/16/2016 02:19 AM, Augie Fackler wrote:


On Nov 24, 2016, at 10:28 AM, FUJIWARA Katsunori
> wrote:

Yes, "files:" was the original version of this patch
and the case I really
care about :) I changed it to rootglob after your
comments.
Which way would be preferred to move forward?


"files:" is "path:" family, and "rootglob:" is "glob:"
family. As we
concluded before, "path:" itself can't control recursion of
matching
well.

Therefore, I think that "files:" should be implemented if
needed,
regardless of implementing "rootglob:".

Of course, we need high point view of this area, at first :-)


BTW, it is a little ambiguous (at least, for me) that
"files:foo"
matches against both file "foo" and files just under directory
"foo". Name other than "files:" may resolve this ambiguity,
but I
don't have any better (and short enough) name :-<

 ==  === ===
 patternfoo  foo/bar foo/bar/baz
 ==  === ===
 path:fooo o o

 files:foo   o o x

 file:fooo x x
 dir:foo x o o
 ==  === ===


Scanning the plan page, I see that there’s a *lot* of work that
could be done and no consensus as yet, but that the only
immediate use case seems to be the rootfile/rootglob case. Is
there some path forward we could agree on that would unblock
those immediate needs for narrowhg and not make things harder in
the future?

Alternatively, would we be okay with a slight refactor of the
matcher so that narrowhg can introduce a custom filesonly:
matcher for the time being so we can keep making forward
progress there?  I don’t know the matcher code well enough to be
able to guess if this is a reasonable path so we can be unblocked.

(It’s very hard for to justify the amount of work implied by
reaching consensus on FileNamePatternsPlan and then executing
the entire thing when what we need is solvable today with a
sub-hour patch to existing code, thus my trying to find a
solution we can all live with.)


As far as I understand, Foozy finding shows that the feature
narrowhg needs is already there and nothing new is necessary.

You can add "set:" in front of your glob to make it non recursive in
all cases "set:your/directory/you/want/to/match/files/in/*"

If this does not fits your needs, this probably mean I got your
usecase wrong. In that case can you re-explain the issue you are
trying to solve here?


At the project level, it will make sense to clean up the Pattern
Matching at some point, and Foozy wiki work will help us to do that.

Cheers.

--
Pierre-Yves David




--
Pierre-Yves David
___
Mercurial-devel mailing list

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

Unfortunately, while set would match the right files, because of the way
the code is structured, it provides no way to not try visiting the
directories inside the non-recursive match - the set needs to first collect
all the files in all subdirectories (match.py, _expandset) and then filter
that down to the desired ones. In plain hg repos, that's just much slower -
in the context of narrowhg, the repo will simply not have the manifests for
those subdirectories and trying to visit them will crash.

The follow-up change to this one (which I haven't sent yet but is written)
is updating visitdir to allow non-recursiveness, which btw makes something
like "hg files -I rootglob:browser/*" about 4-5x faster in the firefox repo.


On Fri, Dec 16, 2016 at 6:21 AM, Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

>
>
> On 12/16/2016 02:19 AM, Augie Fackler wrote:
>
>>
>> On Nov 24, 2016, at 10:28 AM, FUJIWARA Katsunori 
>>> wrote:
>>>
>>> Yes, "files:" was the original version of this patch and the case I
> really
> care about :) I changed it to rootglob after your comments.
> Which way would be preferred to move forward?
>

>>> "files:" is "path:" family, and "rootglob:" is "glob:" family. As we
>>> concluded before, "path:" itself can't control recursion of matching
>>> well.
>>>
>>> Therefore, I think that "files:" should be implemented if needed,
>>> regardless of implementing "rootglob:".
>>>
>>> Of course, we need high point view of this area, at first :-)
>>>
>>>
>>> BTW, it is a little ambiguous (at least, for me) that "files:foo"
>>> matches against both file "foo" and files just under directory
>>> "foo". Name other than "files:" may resolve this ambiguity, but I
>>> don't have any better (and short enough) name :-<
>>>
>>>  ==  === ===
>>>  patternfoo  foo/bar foo/bar/baz
>>>  ==  === ===
>>>  path:fooo o o
>>>
>>>  files:foo   o o x
>>>
>>>  file:fooo x x
>>>  dir:foo x o o
>>>  ==  === ===
>>>
>>>
>> Scanning the plan page, I see that there’s a *lot* of work that could be
>> done and no consensus as yet, but that the only immediate use case seems to
>> be the rootfile/rootglob case. Is there some path forward we could agree on
>> that would unblock those immediate needs for narrowhg and not make things
>> harder in the future?
>>
>> Alternatively, would we be okay with a slight refactor of the matcher so
>> that narrowhg can introduce a custom filesonly: matcher for the time being
>> so we can keep making forward progress there?  I don’t know the matcher
>> code well enough to be able to guess if this is a reasonable path so we can
>> be unblocked.
>>
>> (It’s very hard for to justify the amount of work implied by reaching
>> consensus on FileNamePatternsPlan and then executing the entire thing when
>> what we need is solvable today with a sub-hour patch to existing code, thus
>> my trying to find a solution we can all live with.)
>>
>
> As far as I understand, Foozy finding shows that the feature narrowhg
> needs is already there and nothing new is necessary.
>
> You can add "set:" in front of your glob to make it non recursive in all
> cases "set:your/directory/you/want/to/match/files/in/*"
>
> If this does not fits your needs, this probably mean I got your usecase
> wrong. In that case can you re-explain the issue you are trying to solve
> here?
>
>
> At the project level, it will make sense to clean up the Pattern Matching
> at some point, and Foozy wiki work will help us to do that.
>
> Cheers.
>
> --
> Pierre-Yves David
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[FUJIWARA Katsunori]

At Wed, 23 Nov 2016 19:55:16 -0800,
Rodrigo Damazio wrote:
> 
> Hi guys - any comments on the preferred way forward?
> 
> (I do have a follow-up patch for optimizing visitdir accordingly, but don't
> want to send it until this one is agreed upon)

Sorry for long interval !

> On Thu, Nov 17, 2016 at 1:19 PM, Rodrigo Damazio 
> wrote:
> 
> >
> >
> > On Thu, Nov 17, 2016 at 7:52 AM, FUJIWARA Katsunori  > > wrote:
> >
> >>
> >> (sorry for late reply)
> >>
> >> At Wed, 26 Oct 2016 14:02:48 -0700,
> >> Rodrigo Damazio wrote:
> >> >
> >> > On Wed, Oct 26, 2016 at 12:17 AM, FUJIWARA Katsunori <
> >> fo...@lares.dti.ne.jp>
> >> > wrote:
> >> >
> >> > >
> >> > > At Tue, 25 Oct 2016 19:51:59 -0700,
> >> > > Rodrigo Damazio wrote:
> >> > > >
> >> > > > On Tue, Oct 25, 2016 at 4:31 PM, FUJIWARA Katsunori <
> >> > > fo...@lares.dti.ne.jp>
> >> > > > wrote:
> >> > > >
> >> > > > >
> >> > > > > At Mon, 24 Oct 2016 10:34:52 -0700,
> >> > > > > Rodrigo Damazio wrote:
> >>
> >> [snip]
> >>
> >> > > > On the other hand, you assume that newly introduced *path syntaxes
> >> > > > > will be recursive, as below. Would you assume that default
> >> > > > > recursive-ness is different between *glob and *path syntaxes ?
> >> > > > >
> >> > > >
> >> > > > path would be recursive, as will glob that ends with ** or regex
> >> that
> >> > > ends
> >> > > > with .*
> >> > > >
> >> > > >
> >> > > > > > Also, for discussion: I assume the *path patterns will be
> >> recursive
> >> > > when
> >> > > > > > they reference a directory. Do we also want a non-recursive
> >> > > equivalent
> >> > > > > > (rootexact, rootfiles, rootnonrecursive or something like that)?
> >> > >
> >> > > How about adding syntax type "file"/"dir" ?
> >> > >
> >> > >   = = =
> >> > >   type  for recursive for non-recursive
> >> > >   = = =
> >> > >   glob  use "**"  use "*"
> >> > >   reomit "$"  append "$"
> >> > >   path  always(*1)
> >> > >   file    always
> >> > >   dir   always(*2)
> >> > >   = = =
> >> > >
> >> > >   (*1) match against both file and directory
> >> > >   (*2) match against only directory
> >> > >
> >> > > "dir" might be overkill, though :-) (is it useful in resolving name
> >> > > collision at merging or so ?)
> >> > >
> >> >
> >> > foozy, thanks so much for the review and discussion.
> >> > Sounds like we do agree about the glob behavior then, so let me know if
> >> > you'd like any changes to the latest version of this patch, other than
> >> > improving documentation. I'm happy to send an updated version as soon as
> >> > someone is ready to review.
> >> >
> >> > I understand the difference between dir and path (and between the
> >> original
> >> > version of this patch and file) would be that they'd validate the type
> >> of
> >> > entry being matched (so that passing a filename to dir or dir name to
> >> file
> >> > would be an error) - is that what you have in mind?
> >>
> >> Yes > "passing a filename to dir or dir name to file would be an error"
> >>
> >>
> >> > The current matchers
> >> > don't have a good mechanism to verify the type, so some significant
> >> > rewiring would need to be done to pass that information down.
> >>
> >> Current match implement uses two additional pattern suffix '(?:/|$)'
> >> and '$' to control recursive matching of "glob" and "path". The former
> >> allows to match recursively (for "glob" and "path"), and the latter
> >> doesn't (only for "glob").
> >>
> >> I simply think using this technique to implement pattern types "file"
> >> and "dir".
> >>
> >> path:PATTERN => ESCAPED-PATTERN(?:/|$)
> >> file:PATTERN => ESCAPED-PATTERN$
> >> dif:PATTERN  => ESCAPED-PATTERN/
> >>
> >
> > Yes, "files:" was the original version of this patch and the case I really
> > care about :) I changed it to rootglob after your comments.
> > Which way would be preferred to move forward?

"files:" is "path:" family, and "rootglob:" is "glob:" family. As we
concluded before, "path:" itself can't control recursion of matching
well.

Therefore, I think that "files:" should be implemented if needed,
regardless of implementing "rootglob:".

Of course, we need high point view of this area, at first :-)


BTW, it is a little ambiguous (at least, for me) that "files:foo"
matches against both file "foo" and files just under directory
"foo". Name other than "files:" may resolve this ambiguity, but I
don't have any better (and short enough) name :-<

  ==  === ===
  patternfoo  foo/bar foo/bar/baz
  ==  === ===
  path:fooo o o

  files:foo   o o x

  file:fooo x x
  dir:foo x o o
  ==  === ===


--
[FUJIWARA Katsunori] 

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

Hi guys - any comments on the preferred way forward?

(I do have a follow-up patch for optimizing visitdir accordingly, but don't
want to send it until this one is agreed upon)


On Thu, Nov 17, 2016 at 1:19 PM, Rodrigo Damazio 
wrote:

>
>
> On Thu, Nov 17, 2016 at 7:52 AM, FUJIWARA Katsunori  > wrote:
>
>>
>> (sorry for late reply)
>>
>> At Wed, 26 Oct 2016 14:02:48 -0700,
>> Rodrigo Damazio wrote:
>> >
>> > On Wed, Oct 26, 2016 at 12:17 AM, FUJIWARA Katsunori <
>> fo...@lares.dti.ne.jp>
>> > wrote:
>> >
>> > >
>> > > At Tue, 25 Oct 2016 19:51:59 -0700,
>> > > Rodrigo Damazio wrote:
>> > > >
>> > > > On Tue, Oct 25, 2016 at 4:31 PM, FUJIWARA Katsunori <
>> > > fo...@lares.dti.ne.jp>
>> > > > wrote:
>> > > >
>> > > > >
>> > > > > At Mon, 24 Oct 2016 10:34:52 -0700,
>> > > > > Rodrigo Damazio wrote:
>>
>> [snip]
>>
>> > > > On the other hand, you assume that newly introduced *path syntaxes
>> > > > > will be recursive, as below. Would you assume that default
>> > > > > recursive-ness is different between *glob and *path syntaxes ?
>> > > > >
>> > > >
>> > > > path would be recursive, as will glob that ends with ** or regex
>> that
>> > > ends
>> > > > with .*
>> > > >
>> > > >
>> > > > > > Also, for discussion: I assume the *path patterns will be
>> recursive
>> > > when
>> > > > > > they reference a directory. Do we also want a non-recursive
>> > > equivalent
>> > > > > > (rootexact, rootfiles, rootnonrecursive or something like that)?
>> > >
>> > > How about adding syntax type "file"/"dir" ?
>> > >
>> > >   = = =
>> > >   type  for recursive for non-recursive
>> > >   = = =
>> > >   glob  use "**"  use "*"
>> > >   reomit "$"  append "$"
>> > >   path  always(*1)
>> > >   file    always
>> > >   dir   always(*2)
>> > >   = = =
>> > >
>> > >   (*1) match against both file and directory
>> > >   (*2) match against only directory
>> > >
>> > > "dir" might be overkill, though :-) (is it useful in resolving name
>> > > collision at merging or so ?)
>> > >
>> >
>> > foozy, thanks so much for the review and discussion.
>> > Sounds like we do agree about the glob behavior then, so let me know if
>> > you'd like any changes to the latest version of this patch, other than
>> > improving documentation. I'm happy to send an updated version as soon as
>> > someone is ready to review.
>> >
>> > I understand the difference between dir and path (and between the
>> original
>> > version of this patch and file) would be that they'd validate the type
>> of
>> > entry being matched (so that passing a filename to dir or dir name to
>> file
>> > would be an error) - is that what you have in mind?
>>
>> Yes > "passing a filename to dir or dir name to file would be an error"
>>
>>
>> > The current matchers
>> > don't have a good mechanism to verify the type, so some significant
>> > rewiring would need to be done to pass that information down.
>>
>> Current match implement uses two additional pattern suffix '(?:/|$)'
>> and '$' to control recursive matching of "glob" and "path". The former
>> allows to match recursively (for "glob" and "path"), and the latter
>> doesn't (only for "glob").
>>
>> I simply think using this technique to implement pattern types "file"
>> and "dir".
>>
>> path:PATTERN => ESCAPED-PATTERN(?:/|$)
>> file:PATTERN => ESCAPED-PATTERN$
>> dif:PATTERN  => ESCAPED-PATTERN/
>>
>
> Yes, "files:" was the original version of this patch and the case I really
> care about :) I changed it to rootglob after your comments.
> Which way would be preferred to move forward?
>
>


smime.p7s
Description: S/MIME Cryptographic Signature
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[FUJIWARA Katsunori]

(sorry for late reply)

At Wed, 26 Oct 2016 14:02:48 -0700,
Rodrigo Damazio wrote:
> 
> On Wed, Oct 26, 2016 at 12:17 AM, FUJIWARA Katsunori 
> wrote:
> 
> >
> > At Tue, 25 Oct 2016 19:51:59 -0700,
> > Rodrigo Damazio wrote:
> > >
> > > On Tue, Oct 25, 2016 at 4:31 PM, FUJIWARA Katsunori <
> > fo...@lares.dti.ne.jp>
> > > wrote:
> > >
> > > >
> > > > At Mon, 24 Oct 2016 10:34:52 -0700,
> > > > Rodrigo Damazio wrote:

[snip]

> > > On the other hand, you assume that newly introduced *path syntaxes
> > > > will be recursive, as below. Would you assume that default
> > > > recursive-ness is different between *glob and *path syntaxes ?
> > > >
> > >
> > > path would be recursive, as will glob that ends with ** or regex that
> > ends
> > > with .*
> > >
> > >
> > > > > Also, for discussion: I assume the *path patterns will be recursive
> > when
> > > > > they reference a directory. Do we also want a non-recursive
> > equivalent
> > > > > (rootexact, rootfiles, rootnonrecursive or something like that)?
> >
> > How about adding syntax type "file"/"dir" ?
> >
> >   = = =
> >   type  for recursive for non-recursive
> >   = = =
> >   glob  use "**"  use "*"
> >   reomit "$"  append "$"
> >   path  always(*1)
> >   file    always
> >   dir   always(*2)
> >   = = =
> >
> >   (*1) match against both file and directory
> >   (*2) match against only directory
> >
> > "dir" might be overkill, though :-) (is it useful in resolving name
> > collision at merging or so ?)
> >
> 
> foozy, thanks so much for the review and discussion.
> Sounds like we do agree about the glob behavior then, so let me know if
> you'd like any changes to the latest version of this patch, other than
> improving documentation. I'm happy to send an updated version as soon as
> someone is ready to review.
> 
> I understand the difference between dir and path (and between the original
> version of this patch and file) would be that they'd validate the type of
> entry being matched (so that passing a filename to dir or dir name to file
> would be an error) - is that what you have in mind?

Yes > "passing a filename to dir or dir name to file would be an error"


> The current matchers
> don't have a good mechanism to verify the type, so some significant
> rewiring would need to be done to pass that information down.

Current match implement uses two additional pattern suffix '(?:/|$)'
and '$' to control recursive matching of "glob" and "path". The former
allows to match recursively (for "glob" and "path"), and the latter
doesn't (only for "glob").

I simply think using this technique to implement pattern types "file"
and "dir".

path:PATTERN => ESCAPED-PATTERN(?:/|$)
file:PATTERN => ESCAPED-PATTERN$
dif:PATTERN  => ESCAPED-PATTERN/


> Another thought is that by supporting file and dir, you're incentivizing
> developers to rely on smarter name collision support (and also case
> collisions) - one could argue that there's no reason for the complexity
> caused by that.

--
[FUJIWARA Katsunori] fo...@lares.dti.ne.jp
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

Sending updated patch via pushgate (description changed).


On Mon, Oct 24, 2016 at 10:34 AM, Rodrigo Damazio 
wrote:

> It sounds like we'd like to do 3 somewhat orthogonal things:
> - allow user to specify the directory the pattern is relative to
> (root/cwd/any)
> - allow the user to specify recursiveness/non-recursiveness consistently
> (not covered by the *path patterns, but could be the defined behavior for
> the globs)
> - clean up the matcher API (discussed during Sprint)
>
> Doing all 3 together would probably take some time and a lot of
> back-and-forth, so I'm wondering if it'd be ok to start by updating this
> patch to implement "rootglob" with consistent recursiveness behavior, and
> we can then more slowly add the other patterns and converge on a cleaner
> API?
>
> Also, for discussion: I assume the *path patterns will be recursive when
> they reference a directory. Do we also want a non-recursive equivalent
> (rootexact, rootfiles, rootnonrecursive or something like that)?
>
> Thanks
> Rodrigo
>
>
>
> On Mon, Oct 24, 2016 at 6:21 AM, Pierre-Yves David <
> pierre-yves.da...@ens-lyon.org> wrote:
>
>>
>>
>> On 10/21/2016 05:13 PM, FUJIWARA Katsunori wrote:
>>
>>> At Tue, 18 Oct 2016 10:12:07 -0400,
>>> Augie Fackler wrote:
>>>

 On Tue, Oct 18, 2016 at 9:52 AM, Yuya Nishihara  wrote:

> On Tue, 18 Oct 2016 09:40:36 -0400, Augie Fackler wrote:
>
>> On Oct 18, 2016, at 09:38, Yuya Nishihara  wrote:
>>>
 After coordinating on irc to figure out what this proposal actually
 is, I've noticed that the semantics of this "exact" proposal are
 exactly what "glob" does today, which means (I think) that
 "files:foo/bar" should be representable as "glob:foo/bar/*" - what
 am
 I missing?

>>>
>>> Maybe we want a "glob" relative to the repo root?
>>>
>>
>> As far as I can tell, it already is. "relglob:" is relative to your
>> location in the repo according to the docs.
>>
>
> Unfortunately that isn't.
>
> 'glob:' - a glob relative to cwd
> 'relglob:' - an unrooted glob (*.c matches C files in
> all dirs)
>
> Don't ask me why. ;-)
>

 Oh wat. It looks like narrowhg might change this behavior in narrowed
 repositories, thus my additional confusion.

 Maybe we should add "absglob" that is always repo-root-absolute. How
 do we feel about that overall?

>>>
>>> FYI, current pattern matching is implemented as below. This was
>>> chatted in "non-recursive directory matching" session of 4.0 sprint,
>>> and sorry for my late posting of this translation from
>>> http://d.hatena.ne.jp/flying-foozy/20140107/1389087728 in Japanese, as
>>> my backlog of the last sprint.
>>>
>>>    === === ===
>>>   pattern type root-ed cwd-ed  any-of-path
>>>    === === ===
>>>   wildcard --- globrelglob
>>>   regexp   re  --- relre
>>>   raw string   pathrelpath ---
>>>    === === ===
>>>
>>>   If rule is read in from file (e.g. .hgignore):
>>>
>>> * "glob" is treated as "relglob"
>>> * "re" is treated as "relre"
>>>
>>>   This is mentioned in "hg help patterns" and "hg help hgignore", but
>>>   syntax name "relglob" and "relre" themselves aren't explained.
>>>
>>>   "end of name" matching is required:
>>>
>>> * for glob/relglob as PATTERN (e.g. argument in command line), but
>>> * not for glob/relglob as INCLUDES/EXCLUDES, or other pattern
>>> syntaxes
>>>
>>>   For example, file "foo/bar/baz" is:
>>>
>>> * not matched at "hg files glob:foo/bar"
>>> * but matched at "hg file -I glob:foo/bar"
>>>
>>>   This isn't mentioned in any help document :-<, and the latter seems
>>>   to cause the issue mentioned in this patch series.
>>>
>>> How about introducing new systematic names like below to re-organize
>>> current complicated mapping between names and matching ? (and enable
>>> "end of name" matching by "-eon" suffix or so)
>>>
>>>     === ===
>>>   pattern type root-ed  cwd-ed  any-of-path
>>>     === ===
>>>   wildcard rootglob cwdglob anyglob
>>>   regexp   rootre   cwdre   anyre
>>>   raw string   rootpath cwdpath anypath
>>>     === ===
>>>
>>
>> Moving toward a more regular and clear feature set and naming seems a
>> win. I'm +1 for moving in that direction.
>>
>> Cheers,
>>
>> --
>> Pierre-Yves David
>>
>
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

It sounds like we'd like to do 3 somewhat orthogonal things:
- allow user to specify the directory the pattern is relative to
(root/cwd/any)
- allow the user to specify recursiveness/non-recursiveness consistently
(not covered by the *path patterns, but could be the defined behavior for
the globs)
- clean up the matcher API (discussed during Sprint)

Doing all 3 together would probably take some time and a lot of
back-and-forth, so I'm wondering if it'd be ok to start by updating this
patch to implement "rootglob" with consistent recursiveness behavior, and
we can then more slowly add the other patterns and converge on a cleaner
API?

Also, for discussion: I assume the *path patterns will be recursive when
they reference a directory. Do we also want a non-recursive equivalent
(rootexact, rootfiles, rootnonrecursive or something like that)?

Thanks
Rodrigo



On Mon, Oct 24, 2016 at 6:21 AM, Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

>
>
> On 10/21/2016 05:13 PM, FUJIWARA Katsunori wrote:
>
>> At Tue, 18 Oct 2016 10:12:07 -0400,
>> Augie Fackler wrote:
>>
>>>
>>> On Tue, Oct 18, 2016 at 9:52 AM, Yuya Nishihara  wrote:
>>>
 On Tue, 18 Oct 2016 09:40:36 -0400, Augie Fackler wrote:

> On Oct 18, 2016, at 09:38, Yuya Nishihara  wrote:
>>
>>> After coordinating on irc to figure out what this proposal actually
>>> is, I've noticed that the semantics of this "exact" proposal are
>>> exactly what "glob" does today, which means (I think) that
>>> "files:foo/bar" should be representable as "glob:foo/bar/*" - what am
>>> I missing?
>>>
>>
>> Maybe we want a "glob" relative to the repo root?
>>
>
> As far as I can tell, it already is. "relglob:" is relative to your
> location in the repo according to the docs.
>

 Unfortunately that isn't.

 'glob:' - a glob relative to cwd
 'relglob:' - an unrooted glob (*.c matches C files in all
 dirs)

 Don't ask me why. ;-)

>>>
>>> Oh wat. It looks like narrowhg might change this behavior in narrowed
>>> repositories, thus my additional confusion.
>>>
>>> Maybe we should add "absglob" that is always repo-root-absolute. How
>>> do we feel about that overall?
>>>
>>
>> FYI, current pattern matching is implemented as below. This was
>> chatted in "non-recursive directory matching" session of 4.0 sprint,
>> and sorry for my late posting of this translation from
>> http://d.hatena.ne.jp/flying-foozy/20140107/1389087728 in Japanese, as
>> my backlog of the last sprint.
>>
>>    === === ===
>>   pattern type root-ed cwd-ed  any-of-path
>>    === === ===
>>   wildcard --- globrelglob
>>   regexp   re  --- relre
>>   raw string   pathrelpath ---
>>    === === ===
>>
>>   If rule is read in from file (e.g. .hgignore):
>>
>> * "glob" is treated as "relglob"
>> * "re" is treated as "relre"
>>
>>   This is mentioned in "hg help patterns" and "hg help hgignore", but
>>   syntax name "relglob" and "relre" themselves aren't explained.
>>
>>   "end of name" matching is required:
>>
>> * for glob/relglob as PATTERN (e.g. argument in command line), but
>> * not for glob/relglob as INCLUDES/EXCLUDES, or other pattern syntaxes
>>
>>   For example, file "foo/bar/baz" is:
>>
>> * not matched at "hg files glob:foo/bar"
>> * but matched at "hg file -I glob:foo/bar"
>>
>>   This isn't mentioned in any help document :-<, and the latter seems
>>   to cause the issue mentioned in this patch series.
>>
>> How about introducing new systematic names like below to re-organize
>> current complicated mapping between names and matching ? (and enable
>> "end of name" matching by "-eon" suffix or so)
>>
>>     === ===
>>   pattern type root-ed  cwd-ed  any-of-path
>>     === ===
>>   wildcard rootglob cwdglob anyglob
>>   regexp   rootre   cwdre   anyre
>>   raw string   rootpath cwdpath anypath
>>     === ===
>>
>
> Moving toward a more regular and clear feature set and naming seems a win.
> I'm +1 for moving in that direction.
>
> Cheers,
>
> --
> Pierre-Yves David
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Pierre-Yves David]

On 10/22/2016 11:35 AM, Yuya Nishihara wrote:

On Thu, 20 Oct 2016 09:19:24 -0700, Rodrigo Damazio wrote:

The issue is that glob:foo/* is recursive in some cases - e.g. "hg files -I
glob:contrib/*" in the hg repo gives me subdirectories of contrib
recursively (including e.g. contrib/docker/apache-server, two levels down).
After discussing a bit more offline with Martin: I'll check if that's a bug
in the matcher's visitdir (rather than a design limitation of glob) before
following up on this change.


It appears -I designed to include subdirectories.

https://selenic.com/repo/hg/file/3.9.2/mercurial/match.py#l135


So the solution to Rodrigo usecase would be a "--include-exact" flag?

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[FUJIWARA Katsunori]

At Tue, 18 Oct 2016 10:12:07 -0400,
Augie Fackler wrote:
> 
> On Tue, Oct 18, 2016 at 9:52 AM, Yuya Nishihara  wrote:
> > On Tue, 18 Oct 2016 09:40:36 -0400, Augie Fackler wrote:
> >> > On Oct 18, 2016, at 09:38, Yuya Nishihara  wrote:
> >> >> After coordinating on irc to figure out what this proposal actually
> >> >> is, I've noticed that the semantics of this "exact" proposal are
> >> >> exactly what "glob" does today, which means (I think) that
> >> >> "files:foo/bar" should be representable as "glob:foo/bar/*" - what am
> >> >> I missing?
> >> >
> >> > Maybe we want a "glob" relative to the repo root?
> >>
> >> As far as I can tell, it already is. "relglob:" is relative to your
> >> location in the repo according to the docs.
> >
> > Unfortunately that isn't.
> >
> > 'glob:' - a glob relative to cwd
> > 'relglob:' - an unrooted glob (*.c matches C files in all 
> > dirs)
> >
> > Don't ask me why. ;-)
> 
> Oh wat. It looks like narrowhg might change this behavior in narrowed
> repositories, thus my additional confusion.
> 
> Maybe we should add "absglob" that is always repo-root-absolute. How
> do we feel about that overall?

FYI, current pattern matching is implemented as below. This was
chatted in "non-recursive directory matching" session of 4.0 sprint,
and sorry for my late posting of this translation from
http://d.hatena.ne.jp/flying-foozy/20140107/1389087728 in Japanese, as
my backlog of the last sprint.

   === === ===
  pattern type root-ed cwd-ed  any-of-path
   === === ===
  wildcard --- globrelglob
  regexp   re  --- relre
  raw string   pathrelpath ---
   === === ===

  If rule is read in from file (e.g. .hgignore):

* "glob" is treated as "relglob"
* "re" is treated as "relre"

  This is mentioned in "hg help patterns" and "hg help hgignore", but
  syntax name "relglob" and "relre" themselves aren't explained.

  "end of name" matching is required:

* for glob/relglob as PATTERN (e.g. argument in command line), but
* not for glob/relglob as INCLUDES/EXCLUDES, or other pattern syntaxes

  For example, file "foo/bar/baz" is:

* not matched at "hg files glob:foo/bar"
* but matched at "hg file -I glob:foo/bar"

  This isn't mentioned in any help document :-<, and the latter seems
  to cause the issue mentioned in this patch series.

How about introducing new systematic names like below to re-organize
current complicated mapping between names and matching ? (and enable
"end of name" matching by "-eon" suffix or so)

    === ===
  pattern type root-ed  cwd-ed  any-of-path
    === ===
  wildcard rootglob cwdglob anyglob
  regexp   rootre   cwdre   anyre
  raw string   rootpath cwdpath anypath
    === ===

Of course, we should take care of backward compatibility of .hgignore
or so (e.g. config knob to warn/abort for new syntax name in .hgignore).


> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

--
[FUJIWARA Katsunori] fo...@lares.dti.ne.jp
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio via Mercurial-devel]

The issue is that glob:foo/* is recursive in some cases - e.g. "hg files -I
glob:contrib/*" in the hg repo gives me subdirectories of contrib
recursively (including e.g. contrib/docker/apache-server, two levels down).
After discussing a bit more offline with Martin: I'll check if that's a bug
in the matcher's visitdir (rather than a design limitation of glob) before
following up on this change.


On Tue, Oct 18, 2016 at 7:39 AM, Yuya Nishihara  wrote:

> On Tue, 18 Oct 2016 10:12:07 -0400, Augie Fackler wrote:
> > On Tue, Oct 18, 2016 at 9:52 AM, Yuya Nishihara  wrote:
> > > On Tue, 18 Oct 2016 09:40:36 -0400, Augie Fackler wrote:
> > >> > On Oct 18, 2016, at 09:38, Yuya Nishihara  wrote:
> > >> >> After coordinating on irc to figure out what this proposal actually
> > >> >> is, I've noticed that the semantics of this "exact" proposal are
> > >> >> exactly what "glob" does today, which means (I think) that
> > >> >> "files:foo/bar" should be representable as "glob:foo/bar/*" - what
> am
> > >> >> I missing?
> > >> >
> > >> > Maybe we want a "glob" relative to the repo root?
> > >>
> > >> As far as I can tell, it already is. "relglob:" is relative to your
> > >> location in the repo according to the docs.
> > >
> > > Unfortunately that isn't.
> > >
> > > 'glob:' - a glob relative to cwd
> > > 'relglob:' - an unrooted glob (*.c matches C files in
> all dirs)
> > >
> > > Don't ask me why. ;-)
> >
> > Oh wat. It looks like narrowhg might change this behavior in narrowed
> > repositories, thus my additional confusion.
> >
> > Maybe we should add "absglob" that is always repo-root-absolute. How
> > do we feel about that overall?
>
> Sounds good to me.
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Yuya Nishihara]

On Tue, 18 Oct 2016 10:12:07 -0400, Augie Fackler wrote:
> On Tue, Oct 18, 2016 at 9:52 AM, Yuya Nishihara  wrote:
> > On Tue, 18 Oct 2016 09:40:36 -0400, Augie Fackler wrote:
> >> > On Oct 18, 2016, at 09:38, Yuya Nishihara  wrote:
> >> >> After coordinating on irc to figure out what this proposal actually
> >> >> is, I've noticed that the semantics of this "exact" proposal are
> >> >> exactly what "glob" does today, which means (I think) that
> >> >> "files:foo/bar" should be representable as "glob:foo/bar/*" - what am
> >> >> I missing?
> >> >
> >> > Maybe we want a "glob" relative to the repo root?
> >>
> >> As far as I can tell, it already is. "relglob:" is relative to your
> >> location in the repo according to the docs.
> >
> > Unfortunately that isn't.
> >
> > 'glob:' - a glob relative to cwd
> > 'relglob:' - an unrooted glob (*.c matches C files in all 
> > dirs)
> >
> > Don't ask me why. ;-)
> 
> Oh wat. It looks like narrowhg might change this behavior in narrowed
> repositories, thus my additional confusion.
> 
> Maybe we should add "absglob" that is always repo-root-absolute. How
> do we feel about that overall?

Sounds good to me.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Augie Fackler]

On Tue, Oct 18, 2016 at 9:52 AM, Yuya Nishihara  wrote:
> On Tue, 18 Oct 2016 09:40:36 -0400, Augie Fackler wrote:
>> > On Oct 18, 2016, at 09:38, Yuya Nishihara  wrote:
>> >> After coordinating on irc to figure out what this proposal actually
>> >> is, I've noticed that the semantics of this "exact" proposal are
>> >> exactly what "glob" does today, which means (I think) that
>> >> "files:foo/bar" should be representable as "glob:foo/bar/*" - what am
>> >> I missing?
>> >
>> > Maybe we want a "glob" relative to the repo root?
>>
>> As far as I can tell, it already is. "relglob:" is relative to your
>> location in the repo according to the docs.
>
> Unfortunately that isn't.
>
> 'glob:' - a glob relative to cwd
> 'relglob:' - an unrooted glob (*.c matches C files in all dirs)
>
> Don't ask me why. ;-)

Oh wat. It looks like narrowhg might change this behavior in narrowed
repositories, thus my additional confusion.

Maybe we should add "absglob" that is always repo-root-absolute. How
do we feel about that overall?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Augie Fackler]

> On Oct 18, 2016, at 09:38, Yuya Nishihara  wrote:
> 
>> After coordinating on irc to figure out what this proposal actually
>> is, I've noticed that the semantics of this "exact" proposal are
>> exactly what "glob" does today, which means (I think) that
>> "files:foo/bar" should be representable as "glob:foo/bar/*" - what am
>> I missing?
> 
> Maybe we want a "glob" relative to the repo root?

As far as I can tell, it already is. "relglob:" is relative to your location in 
the repo according to the docs.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Yuya Nishihara]

On Tue, 18 Oct 2016 09:07:36 -0400, Augie Fackler wrote:
> On Tue, Oct 18, 2016 at 02:46:52PM +0200, Pierre-Yves David wrote:
> > On 10/18/2016 02:25 AM, Augie Fackler wrote:
> > >On Sun, Oct 16, 2016 at 03:50:10PM +0200, Pierre-Yves David wrote:
> > >>
> > >>
> > >>On 10/08/2016 06:58 PM, Rodrigo Damazio Bovendorp via Mercurial-devel 
> > >>wrote:
> > >>># HG changeset patch
> > >>># User Rodrigo Damazio Bovendorp 
> > >>># Date 1475944120 25200
> > >>>#  Sat Oct 08 09:28:40 2016 -0700
> > >>># Node ID 545efe5a72efdce925a6a3fd3774b350c90b5c55
> > >>># Parent  dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
> > >>>match: adding non-recursive directory matching
> > >>>
> > >>>This allows one to match all files in a directory, without matching 
> > >>>anything in subdirectories.
> > >>>It's implemented almost identically to path:, except for the regex 
> > >>>termination, which doesn't
> > >>>allow more than one / after the directory name.
> > >>>
> > >>>diff --git a/mercurial/match.py b/mercurial/match.py
> > >>>--- a/mercurial/match.py
> > >>>+++ b/mercurial/match.py
> > >>>@@ -105,6 +105,9 @@
> > >>> 'glob:' - a glob relative to cwd
> > >>> 're:' - a regular expression
> > >>> 'path:' - a path relative to repository root
> > >>>+'files:' - a path relative to repository root, which is 
> > >>>matched
> > >>>+ non-recursively (files inside the directory 
> > >>>will match,
> > >>>+ but subdirectories and files in them won't
> > >>
> > >>The feature seems useful and we should have it.
> > >>
> > >>The current behavior is a bit strange to me. because we have directory 
> > >>being
> > >>implicitly recursed of just 1 level (directory content). Could we have a
> > >>xxx: where path is never recursed for anything. Listing a directory
> > >>content would be an explicite 'xxx:my/directory/path/*'
> > >>
> > >>We could use 'exact' or 'norecursion' for xxx.
> > >
> > >exact: works for me. I think norecusion: is too long, since users will
> > >need to type this.
> >
> > What about my proposal of changing the semantic to be plain exact matching
> > (no implicit matching of files in a matched directory) ?
> >
> > That would move use to:
> >
> >
> > 'files:' - a path relative to repository root, which is
> >  matched non-recursively
> >
> >   exact:foo → match a file 'foo',
> >   exact:foo/* → match content of 'foo' directory,
> >   exact:foo/** → match anything under 'foo',
> 
> After coordinating on irc to figure out what this proposal actually
> is, I've noticed that the semantics of this "exact" proposal are
> exactly what "glob" does today, which means (I think) that
> "files:foo/bar" should be representable as "glob:foo/bar/*" - what am
> I missing?

Maybe we want a "glob" relative to the repo root?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Augie Fackler]

On Tue, Oct 18, 2016 at 02:46:52PM +0200, Pierre-Yves David wrote:
>
>
> On 10/18/2016 02:25 AM, Augie Fackler wrote:
> >On Sun, Oct 16, 2016 at 03:50:10PM +0200, Pierre-Yves David wrote:
> >>
> >>
> >>On 10/08/2016 06:58 PM, Rodrigo Damazio Bovendorp via Mercurial-devel wrote:
> >>># HG changeset patch
> >>># User Rodrigo Damazio Bovendorp 
> >>># Date 1475944120 25200
> >>>#  Sat Oct 08 09:28:40 2016 -0700
> >>># Node ID 545efe5a72efdce925a6a3fd3774b350c90b5c55
> >>># Parent  dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
> >>>match: adding non-recursive directory matching
> >>>
> >>>This allows one to match all files in a directory, without matching 
> >>>anything in subdirectories.
> >>>It's implemented almost identically to path:, except for the regex 
> >>>termination, which doesn't
> >>>allow more than one / after the directory name.
> >>>
> >>>diff --git a/mercurial/match.py b/mercurial/match.py
> >>>--- a/mercurial/match.py
> >>>+++ b/mercurial/match.py
> >>>@@ -105,6 +105,9 @@
> >>> 'glob:' - a glob relative to cwd
> >>> 're:' - a regular expression
> >>> 'path:' - a path relative to repository root
> >>>+'files:' - a path relative to repository root, which is 
> >>>matched
> >>>+ non-recursively (files inside the directory will 
> >>>match,
> >>>+ but subdirectories and files in them won't
> >>
> >>The feature seems useful and we should have it.
> >>
> >>The current behavior is a bit strange to me. because we have directory being
> >>implicitly recursed of just 1 level (directory content). Could we have a
> >>xxx: where path is never recursed for anything. Listing a directory
> >>content would be an explicite 'xxx:my/directory/path/*'
> >>
> >>We could use 'exact' or 'norecursion' for xxx.
> >
> >exact: works for me. I think norecusion: is too long, since users will
> >need to type this.
>
> What about my proposal of changing the semantic to be plain exact matching
> (no implicit matching of files in a matched directory) ?
>
> That would move use to:
>
>
> 'files:' - a path relative to repository root, which is
>  matched non-recursively
>
>   exact:foo → match a file 'foo',
>   exact:foo/* → match content of 'foo' directory,
>   exact:foo/** → match anything under 'foo',

After coordinating on irc to figure out what this proposal actually
is, I've noticed that the semantics of this "exact" proposal are
exactly what "glob" does today, which means (I think) that
"files:foo/bar" should be representable as "glob:foo/bar/*" - what am
I missing?

(Sorry for not catching this far earlier in this discussion :( )

>
> Cheers,
>
> --
> Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Pierre-Yves David]

On 10/18/2016 02:25 AM, Augie Fackler wrote:

On Sun, Oct 16, 2016 at 03:50:10PM +0200, Pierre-Yves David wrote:



On 10/08/2016 06:58 PM, Rodrigo Damazio Bovendorp via Mercurial-devel wrote:

# HG changeset patch
# User Rodrigo Damazio Bovendorp 
# Date 1475944120 25200
#  Sat Oct 08 09:28:40 2016 -0700
# Node ID 545efe5a72efdce925a6a3fd3774b350c90b5c55
# Parent  dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
match: adding non-recursive directory matching

This allows one to match all files in a directory, without matching anything in 
subdirectories.
It's implemented almost identically to path:, except for the regex termination, 
which doesn't
allow more than one / after the directory name.

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -105,6 +105,9 @@
 'glob:' - a glob relative to cwd
 're:' - a regular expression
 'path:' - a path relative to repository root
+'files:' - a path relative to repository root, which is matched
+ non-recursively (files inside the directory will 
match,
+ but subdirectories and files in them won't


The feature seems useful and we should have it.

The current behavior is a bit strange to me. because we have directory being
implicitly recursed of just 1 level (directory content). Could we have a
xxx: where path is never recursed for anything. Listing a directory
content would be an explicite 'xxx:my/directory/path/*'

We could use 'exact' or 'norecursion' for xxx.


exact: works for me. I think norecusion: is too long, since users will
need to type this.


What about my proposal of changing the semantic to be plain exact 
matching (no implicit matching of files in a matched directory) ?


That would move use to:


'files:' - a path relative to repository root, which is
 matched non-recursively

  exact:foo → match a file 'foo',
  exact:foo/* → match content of 'foo' directory,
  exact:foo/** → match anything under 'foo',

Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] match: adding non-recursive directory matching

[Pierre-Yves David]

On 10/08/2016 06:58 PM, Rodrigo Damazio Bovendorp via Mercurial-devel wrote:

# HG changeset patch
# User Rodrigo Damazio Bovendorp 
# Date 1475944120 25200
#  Sat Oct 08 09:28:40 2016 -0700
# Node ID 545efe5a72efdce925a6a3fd3774b350c90b5c55
# Parent  dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
match: adding non-recursive directory matching

This allows one to match all files in a directory, without matching anything in 
subdirectories.
It's implemented almost identically to path:, except for the regex termination, 
which doesn't
allow more than one / after the directory name.

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -105,6 +105,9 @@
 'glob:' - a glob relative to cwd
 're:' - a regular expression
 'path:' - a path relative to repository root
+'files:' - a path relative to repository root, which is matched
+ non-recursively (files inside the directory will 
match,
+ but subdirectories and files in them won't


The feature seems useful and we should have it.

The current behavior is a bit strange to me. because we have directory 
being implicitly recursed of just 1 level (directory content). Could we 
have a xxx: where path is never recursed for anything. Listing a 
directory content would be an explicite 'xxx:my/directory/path/*'


We could use 'exact' or 'norecursion' for xxx.


 'relglob:' - an unrooted glob (*.c matches C files in all dirs)
 'relpath:' - a path relative to cwd
 'relre:' - a regexp that needn't match the start of a name
@@ -286,7 +289,7 @@
 for kind, pat in [_patsplit(p, default) for p in patterns]:
 if kind in ('glob', 'relpath'):
 pat = pathutil.canonpath(root, cwd, pat, auditor)
-elif kind in ('relglob', 'path'):
+elif kind in ('relglob', 'path', 'files'):
 pat = util.normpath(pat)
 elif kind in ('listfile', 'listfile0'):
 try:
@@ -447,7 +450,8 @@
 if ':' in pattern:
 kind, pat = pattern.split(':', 1)
 if kind in ('re', 'glob', 'path', 'relglob', 'relpath', 'relre',
-'listfile', 'listfile0', 'set', 'include', 'subinclude'):
+'listfile', 'listfile0', 'set', 'include', 'subinclude',
+'files'):
 return kind, pat
 return default, pattern

@@ -540,6 +544,19 @@
 if pat == '.':
 return ''
 return '^' + util.re.escape(pat) + '(?:/|$)'
+if kind == 'files':
+# Match one of:
+# For pat = 'some/dir':
+# some/dir
+# some/dir/
+# some/dir/filename
+# For pat = '' or pat = '.':
+# filename
+if pat == '.':
+escaped = ''
+else:
+escaped = util.re.escape(pat)
+return '^' + escaped + '(?:^|/|$)[^/]*$'
 if kind == 'relglob':
 return '(?:|.*/)' + _globre(pat) + globsuffix
 if kind == 'relpath':
@@ -628,7 +645,7 @@
 break
 root.append(p)
 r.append('/'.join(root) or '.')
-elif kind in ('relpath', 'path'):
+elif kind in ('relpath', 'path', 'files'):
 r.append(pat or '.')
 else: # relglob, re, relre
 r.append('.')
diff --git a/tests/test-locate.t b/tests/test-locate.t
--- a/tests/test-locate.t
+++ b/tests/test-locate.t
@@ -52,6 +52,12 @@
   t/b
   t/e.h
   t/x
+  $ hg locate files:
+  b
+  t.h
+  $ hg locate files:.
+  b
+  t.h
   $ hg locate -r 0 a
   a
   $ hg locate -r 0 NONEXISTENT
@@ -119,6 +125,13 @@
   ../t/e.h (glob)
   ../t/x (glob)

+  $ hg files files:
+  ../b (glob)
+  ../t.h (glob)
+  $ hg files files:.
+  ../b (glob)
+  ../t.h (glob)
+
   $ hg locate b
   ../b (glob)
   ../t/b (glob)
diff --git a/tests/test-walk.t b/tests/test-walk.t
--- a/tests/test-walk.t
+++ b/tests/test-walk.t
@@ -112,6 +112,8 @@
   f  beans/navy  ../beans/navy
   f  beans/pinto ../beans/pinto
   f  beans/turtle../beans/turtle
+  $ hg debugwalk -I 'files:mammals'
+  f  mammals/skunk  skunk
   $ hg debugwalk .
   f  mammals/Procyonidae/cacomistle  Procyonidae/cacomistle
   f  mammals/Procyonidae/coatimundi  Procyonidae/coatimundi
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel



--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH] match: adding non-recursive directory matching

[Rodrigo Damazio Bovendorp via Mercurial-devel]

# HG changeset patch
# User Rodrigo Damazio Bovendorp 
# Date 1475944120 25200
#  Sat Oct 08 09:28:40 2016 -0700
# Node ID 545efe5a72efdce925a6a3fd3774b350c90b5c55
# Parent  dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
match: adding non-recursive directory matching

This allows one to match all files in a directory, without matching anything in 
subdirectories.
It's implemented almost identically to path:, except for the regex termination, 
which doesn't
allow more than one / after the directory name.

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -105,6 +105,9 @@
 'glob:' - a glob relative to cwd
 're:' - a regular expression
 'path:' - a path relative to repository root
+'files:' - a path relative to repository root, which is matched
+ non-recursively (files inside the directory will 
match,
+ but subdirectories and files in them won't
 'relglob:' - an unrooted glob (*.c matches C files in all dirs)
 'relpath:' - a path relative to cwd
 'relre:' - a regexp that needn't match the start of a name
@@ -286,7 +289,7 @@
 for kind, pat in [_patsplit(p, default) for p in patterns]:
 if kind in ('glob', 'relpath'):
 pat = pathutil.canonpath(root, cwd, pat, auditor)
-elif kind in ('relglob', 'path'):
+elif kind in ('relglob', 'path', 'files'):
 pat = util.normpath(pat)
 elif kind in ('listfile', 'listfile0'):
 try:
@@ -447,7 +450,8 @@
 if ':' in pattern:
 kind, pat = pattern.split(':', 1)
 if kind in ('re', 'glob', 'path', 'relglob', 'relpath', 'relre',
-'listfile', 'listfile0', 'set', 'include', 'subinclude'):
+'listfile', 'listfile0', 'set', 'include', 'subinclude',
+'files'):
 return kind, pat
 return default, pattern
 
@@ -540,6 +544,19 @@
 if pat == '.':
 return ''
 return '^' + util.re.escape(pat) + '(?:/|$)'
+if kind == 'files':
+# Match one of:
+# For pat = 'some/dir':
+# some/dir
+# some/dir/
+# some/dir/filename
+# For pat = '' or pat = '.':
+# filename
+if pat == '.':
+escaped = ''
+else:
+escaped = util.re.escape(pat)
+return '^' + escaped + '(?:^|/|$)[^/]*$'
 if kind == 'relglob':
 return '(?:|.*/)' + _globre(pat) + globsuffix
 if kind == 'relpath':
@@ -628,7 +645,7 @@
 break
 root.append(p)
 r.append('/'.join(root) or '.')
-elif kind in ('relpath', 'path'):
+elif kind in ('relpath', 'path', 'files'):
 r.append(pat or '.')
 else: # relglob, re, relre
 r.append('.')
diff --git a/tests/test-locate.t b/tests/test-locate.t
--- a/tests/test-locate.t
+++ b/tests/test-locate.t
@@ -52,6 +52,12 @@
   t/b
   t/e.h
   t/x
+  $ hg locate files:
+  b
+  t.h
+  $ hg locate files:.
+  b
+  t.h
   $ hg locate -r 0 a
   a
   $ hg locate -r 0 NONEXISTENT
@@ -119,6 +125,13 @@
   ../t/e.h (glob)
   ../t/x (glob)
 
+  $ hg files files:
+  ../b (glob)
+  ../t.h (glob)
+  $ hg files files:.
+  ../b (glob)
+  ../t.h (glob)
+
   $ hg locate b
   ../b (glob)
   ../t/b (glob)
diff --git a/tests/test-walk.t b/tests/test-walk.t
--- a/tests/test-walk.t
+++ b/tests/test-walk.t
@@ -112,6 +112,8 @@
   f  beans/navy  ../beans/navy
   f  beans/pinto ../beans/pinto
   f  beans/turtle../beans/turtle
+  $ hg debugwalk -I 'files:mammals'
+  f  mammals/skunk  skunk
   $ hg debugwalk .
   f  mammals/Procyonidae/cacomistle  Procyonidae/cacomistle
   f  mammals/Procyonidae/coatimundi  Procyonidae/coatimundi
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel