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
> 

[PATCH remotenames-ext] display: compute whether the node is obsolete only if needed

[Kostia Balytskyi]

# HG changeset patch
# User Kostia Balytskyi 
# Date 1485532411 28800
#  Fri Jan 27 07:53:31 2017 -0800
# Node ID 494e92ec0cef1e98a5ac0a233d1fc7dcb6ac3fc7
# Parent  ad31e88e4c85ba95642231f73c0915dd6997962f
display: compute whether the node is obsolete only if needed

For unformatted output (e.g. piped somewhere) we don't need
the label information, so we don't need to know whether the node
is obsolete. This is an optimization for repos with many remote
bookmarks.

diff --git a/remotenames.py b/remotenames.py
--- a/remotenames.py
+++ b/remotenames.py
@@ -1207,6 +1207,7 @@ def displayremotebookmarks(ui, repo, opt
 
 # it seems overkill to hide displaying hidden remote bookmarks
 repo = repo.unfiltered()
+useformatted = repo.ui.formatted()
 
 for name in sorted(ns.listnames(repo)):
 node = ns.nodes(repo, name)[0]
@@ -1220,7 +1221,7 @@ def displayremotebookmarks(ui, repo, opt
 fmt = ' ' * padsize + ' %d:%s'
 
 tmplabel = label
-if ctx.obsolete():
+if useformatted and ctx.obsolete():
 tmplabel = tmplabel + ' changeset.obsolete'
 fm.write(color, '%s', name, label=label)
 fm.condwrite(not ui.quiet, 'rev node', fmt, ctx.rev(),
___
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 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
> > >> >>