public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Phillip Wood <phillip.wood123@gmail•com>
To: "D. Ben Knoble" <ben.knoble+github@gmail•com>, git@vger•kernel.org
Cc: Phillip Wood <phillip.wood@dunelm•org.uk>,
	Taylor Blau <me@ttaylorr•com>, Junio C Hamano <gitster@pobox•com>
Subject: Re: [PATCH 1/5] parseopt: fix :(optional) at command line to only ignore missing files
Date: Tue, 4 Nov 2025 16:19:31 +0000	[thread overview]
Message-ID: <ec8d1764-649d-4edf-b0ae-a19ead5f6f9a@gmail.com> (raw)
In-Reply-To: <9ec696eaac647aa01466b101129da2b12ef5dbd5.1762100242.git.ben.knoble+github@gmail.com>

Hi Ben

These all look good to me though I agree with Junio's comments on patch 
3. It would be nice to get at least the fist patch merged in time for 
2.52.0.

Thanks for following up on these

Phillip

On 02/11/2025 16:17, D. Ben Knoble wrote:
> Unlike the configuration option magic, the parseopt code also ignores
> empty files: compare implementations from ccfcaf399f (parseopt: values
> of pathname type can be prefixed with :(optional), 2025-09-28) and
> 749d6d166d (config: values of pathname type can be prefixed with
> :(optional), 2025-09-28).
> 
> Unify the 2 by not ignoring empty files, which is less surprising and
> the intended semantics from the first patch for config.
> 
> Suggested-by: Phillip Wood <phillip.wood@dunelm•org.uk>
> Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail•com>
> ---
>   parse-options.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/parse-options.c b/parse-options.c
> index 5933468c19..6211b55a83 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -226,7 +226,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
>   		if (!value)
>   			is_optional = 0;
>   		value = fix_filename(p->prefix, value);
> -		if (is_optional && is_empty_or_missing_file(value)) {
> +		if (is_optional && is_missing_file(value)) {
>   			free((char *)value);
>   		} else {
>   			FREE_AND_NULL(*(char **)opt->value);

next prev parent reply	other threads:[~2025-11-04 16:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-02 16:17 [PATCH 0/5] Fixes for :(optional) path code D. Ben Knoble
2025-11-02 16:17 ` [PATCH 1/5] parseopt: fix :(optional) at command line to only ignore missing files D. Ben Knoble
2025-11-04 16:19   ` Phillip Wood [this message]
2025-11-04 17:24     ` Junio C Hamano
2025-11-04 17:34       ` Junio C Hamano
2025-11-04 18:24         ` D. Ben Knoble
2025-11-05 16:35         ` Phillip Wood
2025-11-06 17:47           ` Junio C Hamano
2025-11-02 16:17 ` [PATCH 2/5] doc: clarify command equivalence comment D. Ben Knoble
2025-11-02 16:17 ` [PATCH 3/5] parseopt: use boolean type for a simple flag D. Ben Knoble
2025-11-03  5:19   ` Junio C Hamano
2025-11-04 16:21     ` Phillip Wood
2025-11-04 18:22       ` D. Ben Knoble
2025-11-02 16:17 ` [PATCH 4/5] config: " D. Ben Knoble
2025-11-02 16:17 ` [PATCH 5/5] parseopt: restore const qualifier to parsed filename D. Ben Knoble

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ec8d1764-649d-4edf-b0ae-a19ead5f6f9a@gmail.com \
    --to=phillip.wood123@gmail$(echo .)com \
    --cc=ben.knoble+github@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=me@ttaylorr$(echo .)com \
    --cc=phillip.wood@dunelm$(echo .)org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox