Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 10/21] tree_entry_interesting(): fix depth limit with overlapping pathspecs
Date: Thu, 16 Dec 2010 15:31:39 -0800
Message-ID: <7vmxo5l2g4.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1292425376-14550-11-git-send-email-pclouds@gmail.com> (=?utf-8?B?Ik5ndXnhu4VuIFRow6FpIE5n4buNYw==?= Duy"'s message of "Wed\, 15 Dec 2010 22\:02\:45 +0700")

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Suppose we have two pathspecs 'a' and 'a/b' (both are dirs) and depth
> limit 1. In current code, pathspecs are checked in input order. When
> 'a/b' is checked against pathspec 'a', it fails depth limit and
> therefore is excluded, although it should match 'a/b' pathspec.
>
> This patch reorders all pathspecs alphabetically, then teaches
> tree_entry_interesting() to check against the deepest pathspec first,
> so depth limit of a shallower pathspec won't affect a deeper one.

I am quite happy to see where this new round of this series is going so
far.  I however doubt this patch is a right approach for the problem you
are solving, especially because your longer-term (i.e. toward the rest of
the series to patch 21) plan is to allow wildcards [*1*].

One thing I am not clear is what it means to limit the recursion level
when you have wildcards.

One possible definition of interaction between limit and wildcard may be
to count the number of slashes in the part of the path that matches the
wildcarded part of the pathspec, add the number of path components
appended due to the leading directory match, and then subtract the number
of literal slashes in the wildcarded part of the pattern from the above,
and declare that a match is found if the difference is less than the
limit.

E.g. a pathspec element "a/*/x" would match "a/b/c/x", "a/b/c/d/e/x",
"a/b/x/y" and "a/b/x/y/z" without limit, and with the limit of 1:

    a/b/c/x        matches ('*' expands to "b/c")
    a/b/c/d/e/x    no ('*' has to expand to "c/d/e" and needs 2 levels)
    a/b/x/y        matches ('*' expands to "b" costing zero, "/y" needs 1)
    a/b/x/y/z      does not match

Another definition could be to count _only_ the part that is appended by
recursion (i.e. we do not count how many slashes has to match '*' in the
above examples), and as the option is called --depth, it might make more
sense.

In either case, I am not sure if "if it matches the longest pathspec, we
have the answer without looking at shorter ones" would be a good rule to
use.


[Footnote]

*1* In addition, perhaps you may later want to introduce some "negative"
match operators to pathspecs; while I am not particularly fond of that
direction at this moment, I would like to leave the door open for that
possibility, in case it turns out to be a good thing to have.

  reply index

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-15 15:02 [PATCH 00/21] nd/struct-pathspec v2 Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 01/21] Add struct pathspec Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 02/21] diff-no-index: use diff_tree_setup_paths() Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 03/21] Convert struct diff_options to use struct pathspec Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 04/21] tree_entry_interesting(): remove dependency on struct diff_options Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 05/21] Move tree_entry_interesting() to tree-walk.c and export it Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 06/21] glossary: define pathspec Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 07/21] diff-tree: convert base+baselen to writable strbuf Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 08/21] tree_entry_interesting(): refactor into separate smaller functions Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 09/21] tree_entry_interesting(): support depth limit Nguyễn Thái Ngọc Duy
2011-01-28 20:40   ` Junio C Hamano
2011-01-29  3:13     ` Nguyen Thai Ngoc Duy
2011-01-31 20:21       ` [PATCH] tree_entry_interesting(): with no pathspecs, everything will match Junio C Hamano
2010-12-15 15:02 ` [PATCH 10/21] tree_entry_interesting(): fix depth limit with overlapping pathspecs Nguyễn Thái Ngọc Duy
2010-12-16 23:31   ` Junio C Hamano [this message]
2010-12-17 10:05     ` Nguyen Thai Ngoc Duy
2010-12-17 20:02       ` Junio C Hamano
2010-12-18  3:37     ` Nguyen Thai Ngoc Duy
2010-12-15 15:02 ` [PATCH 12/21] tree_entry_interesting(): optimize wildcard matching when base is matched Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 13/21] pathspec: add match_pathspec_depth() Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 14/21] Convert ce_path_match() to use struct pathspec Nguyễn Thái Ngọc Duy
2010-12-17  0:02   ` Junio C Hamano
2010-12-17  9:59     ` Nguyen Thai Ngoc Duy
2010-12-17 12:43       ` [PATCH 14/21] struct rev_info: convert prune_data to " Nguyễn Thái Ngọc Duy
2010-12-17 12:43         ` [PATCH 15/21] Convert ce_path_match() to use " Nguyễn Thái Ngọc Duy
2010-12-17 15:09       ` [PATCH 14/21] " Junio C Hamano
2010-12-17 15:11         ` Nguyen Thai Ngoc Duy
2010-12-17 20:29           ` Junio C Hamano
2010-12-15 15:02 ` [PATCH 15/21] Convert ce_path_match() to use match_pathspec_depth() Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 16/21] grep: convert to use struct pathspec Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 17/21] grep: use match_pathspec_depth() for cache/worktree grepping Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 18/21] strbuf: allow "buf" to point to the middle of the allocated buffer Nguyễn Thái Ngọc Duy
2010-12-15 15:02 ` [PATCH 19/21] grep: use writable strbuf from caller in grep_tree() Nguyễn Thái Ngọc Duy
2010-12-17  0:15   ` Junio C Hamano
2010-12-15 15:02 ` [PATCH 21/21] t7810: overlapping pathspecs and depth limit Nguyễn Thái Ngọc Duy
2010-12-15 15:02 [PATCH 11/21] tree_entry_interesting(): support wildcard matching Nguyễn Thái Ngọc Duy
2019-02-04 10:36 ` [PATCH] diff-tree doc: correct & remove wrong documentation Ævar Arnfjörð Bjarmason
2019-02-04 10:42   ` Duy Nguyen
2019-02-04 21:10   ` Junio C Hamano
2019-02-04 21:49     ` Ævar Arnfjörð Bjarmason
2010-12-15 15:02 [PATCH 20/21] grep: drop pathspec_matches() in favor of tree_entry_interesting() Nguyễn Thái Ngọc Duy
2010-12-17 12:45 ` Nguyễn Thái Ngọc Duy
2010-12-17  9:56 [PATCH 19/21] grep: use writable strbuf from caller in grep_tree() Nguyen Thai Ngoc Duy
2010-12-17 12:44 ` [PATCH 19/21] grep: use writable strbuf from caller for grep_tree() Nguyễn Thái Ngọc Duy

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vmxo5l2g4.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git