Re: Discussion on git-refs list Implementation and Possible Approaches

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 4, 2025, 11:15 UTC
Message-ID: <Z--_TvQ9MXgjxqOV@pks.im>
In-Reply-To: <20250403154404.3459805-1-05ZYT30@gmail.com>

On Thu, Apr 03, 2025 at 11:44:04PM +0800, Zheng Yuting wrote:

Show 40 quoted lines

> 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>`
>      - `--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.
> 
> **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.

Yup, makes sense.

Another factor is the default format that these two commands use which differs. I would heavily lean towards using the format exposed by `git show-ref` because it doesn't require us to hit the ODB, and thus it is way more efficient. This has bitten me quite often already.

Show 14 quoted lines

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

I don't think it would, both are orthogonal to one another. I don't think people _only_ want to format or _only_ want to filter. Quite often, they'll want to do both at the same time.

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

Why is that? git-for-each-ref(1) already knows to filter and format, so I'd expect the performance to be roughly the same. In fact, I think we would be able to improve performance if we changed the default format as mentioned above.

Show 10 quoted lines

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

I don't think this plan would make sense as it would mean that current users of git-for-each-ref(1) wouldn't be able to migrate.

Patrick