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

Re: [PATCH 1/2] gpg-interface: refactor 'enum sign_mode' parsing

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 11, 2025, 06:06 UTC
Message-ID
<aMJm57VmWb3h167z@pks.im>
In-Reply-To
<20250910080839.2142651-2-christian.couder@gmail.com>
On Wed, Sep 10, 2025 at 10:08:38AM +0200, Christian Couder wrote:
Show 33 quoted lines
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index c06ee0b213..3994a8f898 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -59,24 +57,17 @@ static struct hashmap anonymized_seeds;
>  static struct revision_sources revision_sources;
>  
>  static int parse_opt_sign_mode(const struct option *opt,
> -				     const char *arg, int unset)
> +			       const char *arg, int unset)
>  {
>  	enum sign_mode *val = opt->value;
> +
>  	if (unset)
>  		return 0;
> -	else if (!strcmp(arg, "abort"))
> -		*val = SIGN_ABORT;
> -	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
> -		*val = SIGN_VERBATIM;
> -	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
> -		*val = SIGN_WARN_VERBATIM;
> -	else if (!strcmp(arg, "warn-strip"))
> -		*val = SIGN_WARN_STRIP;
> -	else if (!strcmp(arg, "strip"))
> -		*val = SIGN_STRIP;
> -	else
> -		return error("Unknown %s mode: %s", opt->long_name, arg);
> -	return 0;
> +
> +	if (!parse_sign_mode(arg, val))
> +		return 0;
> +
> +	return error("Unknown %s mode: %s", opt->long_name, arg);

Would it make sense to maybe reverse the error handling and say something like:

    if (parse_sign_mode(arg, val) < 0)
        return error("Unknown %s mode: %s", opt->long_name, arg);
    return 0;
That reads a bit more natural to me at least.
Show 9 quoted lines
> diff --git a/gpg-interface.h b/gpg-interface.h
> index 60ddf8bbfa..44856cc55f 100644
> --- a/gpg-interface.h
> +++ b/gpg-interface.h
> @@ -104,4 +104,19 @@ int check_signature(struct signature_check *sigc,
>  void print_signature_buffer(const struct signature_check *sigc,
>  			    unsigned flags);
>  
> +/* Modes for --signed-tags=<mode> and --signed-commits=<mode> options */
Nit: let's finish this sentence with a dot.
Show 13 quoted lines
> +enum sign_mode {
> +	SIGN_ABORT,
> +	SIGN_WARN_VERBATIM,
> +	SIGN_VERBATIM,
> +	SIGN_WARN_STRIP,
> +	SIGN_STRIP,
> +};
> +
> +/*
> + * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
> + * otherwise.
> + */
> +int parse_sign_mode(const char *arg, enum sign_mode *mode);
Okay, makes sense.
Patrick
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 21 in “fast-import: start controlling how commit signatures are handled”
  1. 0/2 fast-import: start controlling how commit signatures are handledChristian Couder, Sep 10, 2025
  2. 1/2 gpg-interface: refactor 'enum sign_mode' parsingChristian Couder, Sep 10, 2025
  3. Junio C HamanoSep 10, 2025
  4. Patrick SteinhardtSep 11, 2025
  5. Junio C HamanoSep 11, 2025
  6. Christian CouderSep 12, 2025
  7. Christian CouderSep 12, 2025
  8. 2/2 fast-import: add '--signed-commits=<mode>' optionChristian Couder, Sep 10, 2025
  9. Junio C HamanoSep 10, 2025
  10. Christian CouderSep 12, 2025
  11. Junio C HamanoSep 12, 2025
  12. Christian CouderSep 15, 2025
  13. Junio C HamanoSep 15, 2025
  14. Christian CouderSep 17, 2025
  15. Junio C HamanoSep 10, 2025
  16. Christian CouderSep 12, 2025
  17. Junio C HamanoSep 12, 2025
  18. Patrick SteinhardtSep 11, 2025
  19. Junio C HamanoSep 11, 2025
  20. Christian CouderSep 12, 2025
  21. Christian CouderSep 12, 2025

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.