From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 01 Aug 2025 13:27:47 GMT
Subject: Re: [GSoC][RFC PATCH v4 3/5] builtin/refs: add list subcommand
Message-ID: <2d2f823d-6e85-44a0-85d2-d45d4dc287fc@gmail.com>
In-Reply-To: <20250731090040.1625303-4-meetsoni3017@gmail.com>

Hi Soni

On 31/07/2025 10:00, Meet Soni wrote:
> Git's reference management is distributed across multiple commands. As
> part of an ongoing effort to consolidate and modernize reference
> handling, introduce a `list` subcommand under the `git refs` umbrella as
> a replacement for `git for-each-ref`.
> 
> Implement `cmd_refs_list` by having it call the `for_each_ref_core()`
> helper function. This helper was factored out of the original
> `cmd_for_each_ref` in a preceding commit, allowing both commands to
> share the same core logic as independent peers.
> 
> Add documentation for the new command. The man page leverages the shared
> options file, created in a previous commit, by using the AsciiDoc
> `include::` macro to ensure consistency with git-for-each-ref(1).

I find the behavior of for-each-ref quite confusing so I wonder if we 
really want to copy it to the new command. For example

     git for-each-ref 'refs/h*'

does not print anything but

     git for-each-ref 'refs/heads/m*'

prints all the branches beginning with m. Another example is

     git for-each-ref 'refs/heads'

prints all the branches but

     git for-each-ref 'refs/heads*'

prints nothing. To me it would be much easier to understand if the 
pattern always required an explicit wildcard (or possible always did a 
prefix match in the absence of a wildcard).

Thanks

Phillip

> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: shejialuo <shejialuo@gmail.com>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
> ---
>   Documentation/git-refs.adoc | 16 ++++++++++++++++
>   builtin/refs.c              | 14 ++++++++++++++
>   2 files changed, 30 insertions(+)
> 
> diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
> index 4d6dc994f9..ee563aa7e0 100644
> --- a/Documentation/git-refs.adoc
> +++ b/Documentation/git-refs.adoc
> @@ -11,6 +11,13 @@ SYNOPSIS
>   [synopsis]
>   git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]
>   git refs verify [--strict] [--verbose]
> +git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
> +	      [(--sort=<key>)...] [--format=<format>]
> +	      [--include-root-refs] [ --stdin | <pattern>... ]
> +	      [--points-at=<object>]
> +	      [--merged[=<object>]] [--no-merged[=<object>]]
> +	      [--contains[=<object>]] [--no-contains[=<object>]]
> +	      [--exclude=<pattern> ...]
>   
>   DESCRIPTION
>   -----------
> @@ -26,6 +33,11 @@ migrate::
>   verify::
>   	Verify reference database consistency.
>   
> +list::
> +	List references in the repository with support for filtering,
> +	formatting, and sorting. This subcommand is an alias for
> +	linkgit:git-for-each-ref[1] and offers identical functionality.
> +
>   OPTIONS
>   -------
>   
> @@ -57,6 +69,10 @@ The following options are specific to 'git refs verify':
>   --verbose::
>   	When verifying the reference database consistency, be chatty.
>   
> +The following options are specific to 'git refs list':
> +
> +include::for-each-ref-options.adoc[]
> +
>   KNOWN LIMITATIONS
>   -----------------
>   
> diff --git a/builtin/refs.c b/builtin/refs.c
> index 998d2a2c1c..848a7c9072 100644
> --- a/builtin/refs.c
> +++ b/builtin/refs.c
> @@ -6,6 +6,7 @@
>   #include "refs.h"
>   #include "strbuf.h"
>   #include "worktree.h"
> +#include "for-each-ref.h"
>   
>   #define REFS_MIGRATE_USAGE \
>   	N_("git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]")
> @@ -101,6 +102,17 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix,
>   	return ret;
>   }
>   
> +static int cmd_refs_list(int argc, const char **argv, const char *prefix,
> +			   struct repository *repo)
> +{
> +	static char const * const refs_list_usage[] = {
> +		N_("git refs list " COMMON_USAGE_FOR_EACH_REF),
> +		NULL
> +	};
> +
> +	return for_each_ref_core(argc, argv, prefix, repo, refs_list_usage);
> +}
> +
>   int cmd_refs(int argc,
>   	     const char **argv,
>   	     const char *prefix,
> @@ -109,12 +121,14 @@ int cmd_refs(int argc,
>   	const char * const refs_usage[] = {
>   		REFS_MIGRATE_USAGE,
>   		REFS_VERIFY_USAGE,
> +		"git refs list " COMMON_USAGE_FOR_EACH_REF,
>   		NULL,
>   	};
>   	parse_opt_subcommand_fn *fn = NULL;
>   	struct option opts[] = {
>   		OPT_SUBCOMMAND("migrate", &fn, cmd_refs_migrate),
>   		OPT_SUBCOMMAND("verify", &fn, cmd_refs_verify),
> +		OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
>   		OPT_END(),
>   	};
>