git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Karthik Nayak <karthik.188@gmail.com>
To: Zheng Yuting <05zyt30@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: Discussion on git-refs list Implementation and Possible Approaches
Date: Fri, 4 Apr 2025 04:08:26 -0700	[thread overview]
Message-ID: <CAOLa=ZTTPuNyaE5Z-bfkQougmKQSrRZZwLaxJUL7mdmj8uHoFw@mail.gmail.com> (raw)
In-Reply-To: <20250403154404.3459805-1-05ZYT30@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5578 bytes --]

Zheng Yuting <05zyt30@gmail.com> writes:

> After an initial review of the code and documentation for `git-show-ref`
> and `git-for-each-ref`, I believe the functionality of the `git-refs list`
> subcommand can be categorized into two major types:
>
> 1. **Filtering options**
>    - In `git-for-each-ref`:
>      - `--count`
>      - `--sort=<key>`

I would categorize '--sort' into a third subcategory. Filtering refers
to possible change in the size of the sample set. While sorting is more
of a presentation utility.

>      - `--points-at=<object>`
>      - `--merged[=<object>]`
>      - `--no-merged[=<object>]`
>      - `--contains[=<object>]`
>      - `--no-contains[=<object>]`
>      - `--omit-empty`
>      - `--exclude=<pattern>`
>      - `--include-root-refs`
>    - In `git-show-ref`:
>      - `--head`
>      - `--branches`
>      - `--tags`
>      - `--exclude-existing[=<pattern>]`
>
> 2. **Formatting options**
>    - In `git-for-each-ref`:
>      - `--format=<format>`
>      - `--color[=<when>]`
>      - `--tcl`
>      - `--shell`
>      - `--perl`
>    - In `git-show-ref`:
>      - `--dereference`
>      - `--hash`
>
> Additionally, for filtering functionality, the `--ignore-case` option
> from `git-for-each-ref` should be supported across the board.
>

This is indeed a special case which applies to both sorting and
filtering.

> **Note**: The `--verify`, `--quiet` and `--exist` options in
> `git-show-ref` are intended to be implemented as separate
> `git-refs` subcommands and are not within the scope of this
> discussion.
>
>
> ## Implementation Considerations
>
> At this point, I haven't come up with a perfect implementation
> plan, as each approach has some issues:
>
> ### Approach 1:
> `git-refs list` would support both filtering and formatting options,
> meaning it could provide:
> - Filtered output
> - Formatted output
> - Combined filter + format output
>
> However, I see two potential problems with this approach:
> 1. Would it make the `list` subcommand too complex?

You mean complex from the user perspective of having too many options or
from the implementation perspective.

I think from the UX perspective, it is a good time to rethink usage and
need for the options you mentioned above. , for e.g. with '--format', do
we need to have '--tcl', `--shell` and `--perl`?

> 2. The performance could be worse than `git-for-each-ref`.
>

Why would it be worse? The performance difference between
`git-for-each-ref(1)` and `git-show-ref(1)` stem from the formats they
use by default.

$ hyperfine --shell=none --warmup=3 "git for-each-ref" "git show-ref"
Benchmark 1: git for-each-ref
  Time (mean ± σ):       4.0 ms ±   0.6 ms    [User: 1.9 ms, System: 1.9 ms]
  Range (min … max):     3.0 ms …   5.7 ms    680 runs

Benchmark 2: git show-ref
  Time (mean ± σ):       2.9 ms ±   0.4 ms    [User: 1.2 ms, System: 1.5 ms]
  Range (min … max):     2.0 ms …   4.3 ms    909 runs

Summary
  git show-ref ran
    1.38 ± 0.28 times faster than git for-each-ref

What I found interesting was that changing the format for
'git-for-each-ref(1)' gives it a boost:

$ hyperfine --shell=none --warmup=3 'git for-each-ref
--format="%(objectname) %(refname)"' "git show-ref"
Benchmark 1: git for-each-ref --format="%(objectname) %(refname)"
  Time (mean ± σ):       2.4 ms ±   0.3 ms    [User: 1.1 ms, System: 1.1 ms]
  Range (min … max):     1.7 ms …   3.6 ms    1070 runs

Benchmark 2: git show-ref
  Time (mean ± σ):       2.9 ms ±   0.4 ms    [User: 1.2 ms, System: 1.5 ms]
  Range (min … max):     2.0 ms …   4.5 ms    833 runs

Summary
  git for-each-ref --format="%(objectname) %(refname)" ran
    1.20 ± 0.23 times faster than git show-ref

> ### Approach 2:
> Split the functionality into two separate subcommands:
> - `git-refs filter`: Handles filtering and filter + format output
> - `git-refs show`: Supports formatting options
>
> For implementation, my initial thought is that `git-refs filter` could
> reuse the formatting options from `git-refs show`. Perhaps this could
> work similarly to how `git-add --patch` and `git-restore --patch`
> share logic, though I haven’t thoroughly reviewed that part of the
> code yet. Would this be a reasonable approach?
>

And what is the expectation that when you want to do both filtering and
formatting, would the user be expected to do `git refs filter | git refs
show`? Generally users want to combine both of these options.

Also wasn't the idea to already implement `git-refs show` as a
standalone which simply shows what value a reference holds (without
derefence)?

> ## Overall Plan
>
> If Approach 2 is preferable, I could start with `git-refs show` since it
> only deals with basic ref listing and formatting. I would then make
> the formatting code more reusable to support `git-refs filter`, which
> would focus solely on filtering.
>
> If Approach 1 is chosen, the implementation plan would remain the
> same, but everything would be handled within a single `git-refs list`
> command.

While I would think Approach 1 is the better option here, I'm also
seeing how it is complex, perhaps a good option to get started would be
to implement a simpler subcommand as a first case? Perhaps the
originally discussed `git refs show`?

>
> I would appreciate any feedback or alternative suggestions on the
> best way to structure this functionality.
>
> Thanks!

Thanks for the proposal!
Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

  reply	other threads:[~2025-04-04 11:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-23 13:36 [GSoC] Proposal Discussion: git-refs Project Yuting Zheng
2025-03-24 12:02 ` Patrick Steinhardt
2025-03-27  2:26   ` Yuting Zheng
2025-03-28 13:45     ` shejialuo
2025-03-29 14:54       ` Yuting Zheng
2025-03-29 15:02 ` [GSoC] git-refs proposal draft Zheng Yuting
2025-03-31  9:42   ` Patrick Steinhardt
2025-04-01 13:37     ` Yuting Zheng
2025-04-02  8:02       ` Patrick Steinhardt
2025-04-03 15:44   ` Discussion on git-refs list Implementation and Possible Approaches Zheng Yuting
2025-04-04 11:08     ` Karthik Nayak [this message]
2025-04-04 15:25       ` Yuting Zheng
2025-04-04 11:15     ` Patrick Steinhardt
     [not found]       ` <CAMvj1+rMY2YR8_GGFeDoJ6HCiVDusZZk9fAguKh=kbctHO=2Qg@mail.gmail.com>
2025-04-04 15:20         ` Fwd: " Yuting Zheng
2025-04-04 15:26       ` Yuting Zheng
2025-04-04 15:16     ` Yuting Zheng
2025-04-06  6:08   ` [GSoC] git-refs proposal v2 Yuting Zheng

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

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

  git send-email \
    --in-reply-to='CAOLa=ZTTPuNyaE5Z-bfkQougmKQSrRZZwLaxJUL7mdmj8uHoFw@mail.gmail.com' \
    --to=karthik.188@gmail.com \
    --cc=05zyt30@gmail.com \
    --cc=git@vger.kernel.org \
    /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.
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 NNTP newsgroup(s).