Re: [PATCH 2/2] branch, for-each-ref: add option to omit empty lines

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Øystein

On 30/03/2023 12:21, Øystein Walle wrote:
> If the given format string expands to the empty string a newline is
> still printed it. This makes using the output linewise more tedious. For
> example, git update-ref --stdin does not accept empty lines.
> 
> Add options to branch and for-each-ref to not print these empty lines.
> The default behavior remains the same.

Do the empty lines in the output serve any useful purpose? If not then 
it might be better just to suppress them unconditionally rather than 
adding a new command line option.

Best Wishes

Phillip

> Signed-off-by: Øystein Walle <oystwa@gmail.com>
> ---
> 
> The logic is more or less duplicated in branch.c and for-each-ref.c
> which I don't like. However I couldn't really find a "central" place to
> put it. Imo. it's definitely not a property of the format or the filter,
> so struct ref_format and struct ref_filter are no good.
> 
> I also started working on a patch to make update-ref --stdin accept
> empty lines. But that seems to be a much more deliberate decision, with
> tests to verify it and all. So I stopped pursuing that.
> 
>   Documentation/git-branch.txt       |  5 +++++
>   Documentation/git-for-each-ref.txt |  5 +++++
>   builtin/branch.c                   | 12 +++++++++++-
>   builtin/for-each-ref.c             | 15 +++++++++++----
>   t/t3203-branch-output.sh           | 26 ++++++++++++++++++++++++++
>   t/t6300-for-each-ref.sh            |  8 ++++++++
>   6 files changed, 66 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index d382ac69f7..4d53133ce3 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -156,6 +156,11 @@ in another worktree linked to the same repository.
>   --ignore-case::
>   	Sorting and filtering branches are case insensitive.
>   
> +-n::
> +--omit-empty-lines::
> +	Do not print a newline after formatted refs where the format expands
> +	to the empty string.
> +
>   --column[=<options>]::
>   --no-column::
>   	Display branch listing in columns. See configuration variable
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 6da899c629..0f4fa98b18 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -93,6 +93,11 @@ OPTIONS
>   --ignore-case::
>   	Sorting and filtering refs are case insensitive.
>   
> +-n::
> +--omit-empty-lines::
> +	Do not print a newline after formatted refs where the format expands
> +	to the empty string.
> +
>   FIELD NAMES
>   -----------
>   
> diff --git a/builtin/branch.c b/builtin/branch.c
> index f63fd45edb..1bbb36b442 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -41,6 +41,7 @@ static const char *head;
>   static struct object_id head_oid;
>   static int recurse_submodules = 0;
>   static int submodule_propagate_branches = 0;
> +static int omit_empty_lines = 0;
>   
>   static int branch_use_color = -1;
>   static char branch_colors[][COLOR_MAXLEN] = {
> @@ -461,7 +462,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
>   			string_list_append(output, out.buf);
>   		} else {
>   			fwrite(out.buf, 1, out.len, stdout);
> -			putchar('\n');
> +			if (!omit_empty_lines || out.len > 0)
> +				putchar('\n');
>   		}
>   	}
>   
> @@ -670,6 +672,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>   		OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
>   		OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
>   		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
> +		OPT_BOOL('n' , "omit-empty-lines",  &omit_empty_lines,
> +			N_("do not output a newline after empty formatted refs")),
>   		OPT_BIT('c', "copy", &copy, N_("copy a branch and its reflog"), 1),
>   		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
>   		OPT_BOOL('l', "list", &list, N_("list branch names")),
> @@ -757,7 +761,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>   	}
>   
>   	if (list)
> +	{
> +		if (omit_empty_lines && !format.format) {
> +			error("--omit-empty-lines without --format does not make sense");
> +			usage_with_options(builtin_branch_usage, options);
> +		}
>   		setup_auto_pager("branch", 1);
> +	}
>   
>   	if (delete) {
>   		if (!argc)
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 6f62f40d12..349c4d4ef8 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -19,7 +19,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>   	int i;
>   	struct ref_sorting *sorting;
>   	struct string_list sorting_options = STRING_LIST_INIT_DUP;
> -	int maxcount = 0, icase = 0;
> +	int maxcount = 0, icase = 0, omit_empty_lines = 0;
>   	struct ref_array array;
>   	struct ref_filter filter;
>   	struct ref_format format = REF_FORMAT_INIT;
> @@ -35,6 +35,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>   			N_("quote placeholders suitably for python"), QUOTE_PYTHON),
>   		OPT_BIT(0 , "tcl",  &format.quote_style,
>   			N_("quote placeholders suitably for Tcl"), QUOTE_TCL),
> +		OPT_BOOL('n' , "omit-empty-lines",  &omit_empty_lines,
> +			N_("do not output a newline after empty formatted refs")),
>   
>   		OPT_GROUP(""),
>   		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
> @@ -55,8 +57,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>   	memset(&array, 0, sizeof(array));
>   	memset(&filter, 0, sizeof(filter));
>   
> -	format.format = "%(objectname) %(objecttype)\t%(refname)";
> -
>   	git_config(git_default_config, NULL);
>   
>   	parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);
> @@ -68,6 +68,12 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>   		error("more than one quoting style?");
>   		usage_with_options(for_each_ref_usage, opts);
>   	}
> +	if (omit_empty_lines && !format.format) {
> +		error("--omit-empty-lines without --format does not make sense");
> +		usage_with_options(for_each_ref_usage, opts);
> +	}
> +	if (!format.format)
> +		format.format = "%(objectname) %(objecttype)\t%(refname)";
>   	if (verify_ref_format(&format))
>   		usage_with_options(for_each_ref_usage, opts);
>   
> @@ -88,7 +94,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>   		if (format_ref_array_item(array.items[i], &format, &output, &err))
>   			die("%s", err.buf);
>   		fwrite(output.buf, 1, output.len, stdout);
> -		putchar('\n');
> +		if (!omit_empty_lines || output.len > 0)
> +			putchar('\n');
>   	}
>   
>   	strbuf_release(&err);
> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index d34d77f893..26bf0819ea 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -341,6 +341,32 @@ test_expect_success 'git branch with --format=%(rest) must fail' '
>   	test_must_fail git branch --format="%(rest)" >actual
>   '
>   
> +test_expect_success 'git branch --format --omit-empty-lines' '
> +	cat >expect <<-\EOF &&
> +	Refname is (HEAD detached from fromtag)
> +	Refname is refs/heads/ambiguous
> +	Refname is refs/heads/branch-one
> +	Refname is refs/heads/branch-two
> +	EOF
> +	echo >>expect &&
> +	cat >>expect <<-\EOF &&
> +	Refname is refs/heads/ref-to-branch
> +	Refname is refs/heads/ref-to-remote
> +	EOF
> +	git branch --format="%(if:notequals=refs/heads/main)%(refname)%(then)Refname is %(refname)%(end)" >actual &&
> +	test_cmp expect actual &&
> +	cat >expect <<-\EOF &&
> +	Refname is (HEAD detached from fromtag)
> +	Refname is refs/heads/ambiguous
> +	Refname is refs/heads/branch-one
> +	Refname is refs/heads/branch-two
> +	Refname is refs/heads/ref-to-branch
> +	Refname is refs/heads/ref-to-remote
> +	EOF
> +	git branch --omit-empty-lines --format="%(if:notequals=refs/heads/main)%(refname)%(then)Refname is %(refname)%(end)" >actual &&
> +	test_cmp expect actual
> +'
> +
>   test_expect_success 'worktree colors correct' '
>   	cat >expect <<-EOF &&
>   	* <GREEN>(HEAD detached from fromtag)<RESET>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index c466fd989f..eec9d45513 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -1374,6 +1374,14 @@ test_expect_success 'for-each-ref --ignore-case ignores case' '
>   	test_cmp expect actual
>   '
>   
> +test_expect_success 'for-each-ref --omit-empty-lines works' '
> +	git for-each-ref --format="%(refname)" > actual &&
> +	test_line_count -gt 1 actual &&
> +	git for-each-ref --format="%(if:equals=refs/heads/main)%(refname)%(then)%(refname)%(end)" --omit-empty-lines > actual &&
> +	echo refs/heads/main > expect &&
> +	test_cmp expect actual
> +'
> +
>   test_expect_success 'for-each-ref --ignore-case works on multiple sort keys' '
>   	# name refs numerically to avoid case-insensitive filesystem conflicts
>   	nr=0 &&