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

Re: [PATCH 3/7] revision: use die_for_incompatible_opt3() for --graph/--reverse/--walk-reflogs

From
René Scharfe <l.s.r@web.de>
Date
Dec 6, 2023, 13:47 UTC
Message-ID
<382fac73-1afc-4b89-ad0f-3320c7a64838@web.de>
In-Reply-To
<ZXByNGuQTaOQ3sKW@tanuki>
Am 06.12.23 um 14:08 schrieb Patrick Steinhardt:
Show 41 quoted lines
> On Wed, Dec 06, 2023 at 12:51:57PM +0100, René Scharfe wrote:
>> The revision options --reverse is incompatibel with --walk-reflogs and
>> --graph is incompatible with both --reverse and --walk-reflogs.  So they
>> are all incompatible with each other.
>>
>> Use the function for checking three mutually incompatible options,
>> die_for_incompatible_opt3(), to perform this check in one place and
>> without repetition.  This is shorter and clearer.
>>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>>  revision.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/revision.c b/revision.c
>> index b2861474b1..1b7e1af6c6 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -3036,8 +3036,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>>  		revs->grep_filter.ignore_locale = 1;
>>  	compile_grep_patterns(&revs->grep_filter);
>>
>> -	if (revs->reverse && revs->reflog_info)
>> -		die(_("options '%s' and '%s' cannot be used together"), "--reverse", "--walk-reflogs");
>>  	if (revs->reflog_info && revs->limited)
>>  		die("cannot combine --walk-reflogs with history-limiting options");
>>  	if (revs->rewrite_parents && revs->children.name)
>> @@ -3048,11 +3046,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>>  	/*
>>  	 * Limitations on the graph functionality
>>  	 */
>> -	if (revs->reverse && revs->graph)
>> -		die(_("options '%s' and '%s' cannot be used together"), "--reverse", "--graph");
>> +	die_for_incompatible_opt3(!!revs->graph, "--graph",
>> +				  !!revs->reverse, "--reverse",
>> +				  !!revs->reflog_info, "--walk-reflogs");
>
> I've been wondering why we use `!!` here, as `die_for_incompatible_*()`
> doesn't care for the actual value but only checks that it is non-zero.
> Is it because of the type mismatch, where these flags here use unsigned
> ints whereas `die_for_incompatible_*()` expect ints?

->graph and ->reflog_info are pointers and clang reports an int-conversion warning without the double negation. ->reverse is an unsigned:1 and so doesn't need it, but I gave it one anyway for aesthetic reasons.

René
Previous: Patrick SteinhardtNext: Eric Sunshine
Message 4 of 20 in “standardize incompatibility messages”
  1. 0/7 standardize incompatibility messagesRené Scharfe, Dec 6, 2023
  2. 3/7 revision: use die_for_incompatible_opt3() for --graph/--reverse/--walk-reflogsRené Scharfe, Dec 6, 2023
  3. Patrick SteinhardtDec 6, 2023
  4. René ScharfeDec 6, 2023
  5. Eric SunshineDec 6, 2023
  6. René ScharfeDec 6, 2023
  7. 4/7 revision, rev-parse: factorize incompatibility messages about --exclude-hiddenRené Scharfe, Dec 6, 2023
  8. Patrick SteinhardtDec 6, 2023
  9. René ScharfeDec 6, 2023
  10. Patrick SteinhardtDec 6, 2023
  11. René ScharfeDec 6, 2023
  12. Taylor BlauDec 6, 2023
  13. Patrick SteinhardtDec 7, 2023
  14. 1/7 push: use die_for_incompatible_opt4() for --delete/--tags/--all/--mirrorRené Scharfe, Dec 6, 2023
  15. 2/7 repack: use die_for_incompatible_opt3() for -A/-k/--cruftRené Scharfe, Dec 6, 2023
  16. Taylor BlauDec 6, 2023
  17. 5/7 clean: factorize incompatibility messageRené Scharfe, Dec 6, 2023
  18. 7/7 worktree: simplify incompatibility message for --orphan and commit-ishRené Scharfe, Dec 6, 2023
  19. 6/7 worktree: standardize incompatibility messagesRené Scharfe, Dec 6, 2023
  20. Patrick SteinhardtDec 6, 2023

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.