git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Teng Long <dyroneteng@gmail.com>
Cc: me@ttaylorr.com, git@vger.kernel.org, tenglong.tl@alibaba-inc.com
Subject: Re: [RFC PATCH 0/6] ls-tree: introduce '--pattern' option
Date: Mon, 21 Nov 2022 13:12:08 +0100	[thread overview]
Message-ID: <221121.8635accxea.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20221121114150.3473-1-tenglong.tl@alibaba-inc.com>


On Mon, Nov 21 2022, Teng Long wrote:

> Taylor Blau <me@ttaylorr.com> writes:
>
>> I think this falls into the same trap as the series on 'git show-ref
>> --count' that I worked on earlier this year [1].
>>
>> At the time, it seemed useful to me (since I was working in an
>> environment where counting the number of lines from 'show-ref' was more
>> cumbersome than teaching 'show-ref' how to perform the same task
>> itself).
>>
>> And I stand by that value judgement, but sharing the patches with the
>> Git mailing list under the intent to merge it in was wrong. Those
>> patches were too niche to be more generally useful, and would only serve
>> to clutter up the UI of show-ref for everybody else.
>>
>> So I was glad to drop that topic. Now, I'd be curious to hear from Teng
>> whether or not there *is* something that we're missing, since if so, I
>> definitely want to know what it is.
>>
>> But absent of that, I tend to agree with Ævar that I'm not compelled by
>> replacing 'ls-tree | grep <pattern>' with 'ls-tree --pattern=<pattern>',
>> especially if the latter is slower than the former.
>>
>> Thanks,
>> Taylor
>>
>> [1]: https://lore.kernel.org/git/cover.1654552560.git.me@ttaylorr.com/
>
> honestly, I just think it's useful to me, but omit the performance recession of
> the option. I originally thought about it looks like the "git tag -l <pattern>"
> or "git branch -l <pattern>" usage, but it seems not as a regex matching on
> them and it indeed executes faster than the pipe grep, because it seems like the
> former has the more restrictive matching conditions (because if I move the
> last aster, there is no output):
>
>
> ✗ git branch -r --list "avar*" | wc -l
>     1498
>
> ✗ hyperfine 'git branch -r --list "avar*"'
> Benchmark 1: git branch -r --list "avar*"
>   Time (mean ± σ):      69.8 ms ±   3.1 ms    [User: 25.8 ms, System: 42.7 ms]
>   Range (min … max):    66.6 ms …  81.8 ms    35 runs
>
> ✗ hyperfine 'git branch -r --list | grep "avar"'
> Benchmark 1: git branch -r --list | grep "avar"
>   Time (mean ± σ):      76.4 ms ±   3.7 ms    [User: 32.7 ms, System: 45.2 ms]
>   Range (min … max):    72.9 ms …  85.5 ms    34 runs
>
> ➜  Documentation git:(tl/extra_bitmap_relative_path) ✗ git branch -r --list "avar" | wc -l
>        0

Yeah, that's the built-in wildmatch() (as in wildmatch.c
in-tree)-powered matching we do, and support in pretty much anything
that takes a <path>.

I *thought* this feature was something like that that when I first
glanced at it, i.e. to regex match ls-tree entries, but e.g. a
--pattern=abc would still match that in the OID, as it's just grepping
the line.

Which I think is one way to draw the "does this belong in git?"
line. I.e. a "grep" or your "--pattern" doesn't need to know anything
about sub-components of the line to be equivalent, whereas something
that just matches the path does.

Also, when you get into e.g. negative pathspecs and the like it becomes
even more compelling.

I do have some WIP patches somewhere to have our pathspecs support PCRE
regexes.

So I think it would be neat, or at least worth exploring. But just
having ls-tree or some random command support it isn't the righ way to
go about it IMO, it should be done via the pathspec API, if done at all.

  reply	other threads:[~2022-11-21 12:20 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-17 11:30 [RFC PATCH 0/6] ls-tree: introduce '--pattern' option Teng Long
2022-11-17 11:30 ` [RFC PATCH 1/6] ls-tree: cleanup the redundant SPACE Teng Long
2022-11-17 11:30 ` [RFC PATCH 2/6] t3104: remove shift code in 'test_ls_tree_format' Teng Long
2022-11-17 11:30 ` [RFC PATCH 3/6] ls-tree: optimize params of 'show_tree_common_default_long()' Teng Long
2022-11-17 11:30 ` [RFC PATCH 4/6] ls-tree: improving cohension in the print code Teng Long
2022-11-17 13:53   ` Ævar Arnfjörð Bjarmason
2022-11-17 11:30 ` [RFC PATCH 5/6] ls-tree: introduce 'match_pattern()' function Teng Long
2022-11-17 14:02   ` Ævar Arnfjörð Bjarmason
2022-11-30  9:39   ` Ævar Arnfjörð Bjarmason
2022-11-17 11:30 ` [RFC PATCH 6/6] ls-tree: introduce '--pattern' option Teng Long
2022-11-17 14:03   ` Ævar Arnfjörð Bjarmason
2022-12-12  8:32   ` Johannes Schindelin
2022-12-12 23:57     ` Junio C Hamano
2022-12-14  5:27       ` Junio C Hamano
2022-12-14 10:03         ` Ævar Arnfjörð Bjarmason
2022-12-14 10:38           ` Junio C Hamano
2023-03-27 10:37       ` win-test: unknown terminal "xterm-256color", was " Johannes Schindelin
2023-03-27 20:42         ` Junio C Hamano
2023-03-28 18:08           ` Jeff King
2023-03-28 19:31             ` Junio C Hamano
2023-03-28 19:59               ` Jeff King
2023-03-28 20:43                 ` Jeff King
2023-03-28 21:05                   ` Junio C Hamano
2022-11-17 13:22 ` [RFC PATCH 0/6] " Ævar Arnfjörð Bjarmason
2022-11-17 22:02   ` Taylor Blau
2022-11-21 11:41     ` Teng Long
2022-11-21 12:12       ` Ævar Arnfjörð Bjarmason [this message]
2022-11-17 13:48 ` [RFC PATCH 0/4] ls-tree: pass state in struct, not globals Ævar Arnfjörð Bjarmason
2022-11-17 13:48   ` [RFC PATCH 1/4] ls-tree: don't use "show_tree_data" for "fast" callbacks Ævar Arnfjörð Bjarmason
2022-12-21 11:47     ` Teng Long
2022-11-17 13:48   ` [RFC PATCH 2/4] ls-tree: use a "struct options" Ævar Arnfjörð Bjarmason
2022-11-17 13:48   ` [RFC PATCH 3/4] ls-tree: fold "show_tree_data" into "cb" struct Ævar Arnfjörð Bjarmason
2022-11-17 13:48   ` [RFC PATCH 4/4] ls-tree: make "line_termination" less generic Ævar Arnfjörð Bjarmason
2022-11-21 12:00   ` [RFC PATCH 0/4] ls-tree: pass state in struct, not globals Teng Long

Reply instructions:

You may reply publicly 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=221121.8635accxea.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=dyroneteng@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=tenglong.tl@alibaba-inc.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).