git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] for-each-ref: fix setup of option-parsing for --sort

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 10, 2007, 18:58 UTC
Message-ID
<7v7ikqlyfo.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<1194713274-31200-1-git-send-email-hjemli@gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
Show 11 quoted lines
> The option value for --sort is already a pointer to a pointer to struct
> ref_sort, so just use it.
>
> Signed-off-by: Lars Hjemli <hjemli@gmail.com>
> ---
>
> On Nov 10, 2007 5:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> Could you add a test for that too, please?
>
> Is this ok?
>

Testing "for that" would be kind of hard and semi pointless, isn't it?

If it's mismatch of the expected number of times a pointer is dereferenced between the caller and the callee, I'd imagine that it will read from and write to random place in memory and would lead to unpredictable behaviour. If you are lucky you would not get expected results but if you are unlucky who knows what would happen.

But the new test makes sure --sort takes intended effect, which is a good thing.

Thanks.
Show 50 quoted lines
>  builtin-for-each-ref.c  |    2 +-
>  t/t6300-for-each-ref.sh |   22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index da8c794..e909e66 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> @@ -847,7 +847,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  		OPT_GROUP(""),
>  		OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
>  		OPT_STRING(  0 , "format", &format, "format", "format to use for the output"),
> -		OPT_CALLBACK(0 , "sort", &sort_tail, "key",
> +		OPT_CALLBACK(0 , "sort", sort_tail, "key",
>  		            "field name to sort on", &opt_parse_sort),
>  		OPT_END(),
>  	};
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index d0809eb..c722635 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -148,4 +148,26 @@ test_expect_success 'Check format "rfc2822" date fields output' '
>  	git diff expected actual
>  '
>  
> +cat >expected <<\EOF
> +refs/heads/master
> +refs/tags/testtag
> +EOF
> +
> +test_expect_success 'Verify ascending sort' '
> +	git-for-each-ref --format="%(refname)" --sort=refname >actual &&
> +	git diff expected actual
> +'
> +
> +
> +cat >expected <<\EOF
> +refs/tags/testtag
> +refs/heads/master
> +EOF
> +
> +test_expect_success 'Verify descending sort' '
> +	git-for-each-ref --format="%(refname)" --sort=-refname >actual &&
> +	git diff expected actual
> +'
> +
> +
>  test_done
> -- 
> 1.5.3.5.623.g0a1d
Previous: Lars HjemliNext: Johannes Schindelin
Message 2 of 3 in “for-each-ref: fix setup of option-parsing for --sort”
  1. for-each-ref: fix setup of option-parsing for --sortLars Hjemli, Nov 10, 2007
  2. Junio C HamanoNov 10, 2007
  3. Johannes SchindelinNov 11, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.