Re: [PATCH v2 0/3] Support :(optional) filepaths

[Junio C Hamano <gitster@pobox.com>]

"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

Before "notes" you would want an overall description of what the
topic is for those who no longer remember the previous iteration,
or for those this iteration is the first one they see.

> Notes:
> - Based on commit 2da08f2c3d (parseopt: values of pathname type can be
>   prefixed with :(optional), 2024-10-14) (broken-out/wip/optional-path)
> - Rebased on v2.51.0

Thanks.

> - I'm least sure of the 3rd patch and am happy to drop it in support of
>   the first 2. I think it might be better to (a) integrate :(optional)
>   support as pathspec magic and (b) use pathspec magic in parse-options
>   when getting filenames. But I'm not sure, and this has other
>   ramifications I'm not prepared to deal with. (For example: `git grep
>   path <file>… :(optional)non-existent` could pretend like
>   `non-existent` was never given?)

While it might not hurt, I do not see a need for such a support.

Pathspec _is_ a pattern.  If an existing path does not match the
pattern, there is no ill effect.  In other words, in this command
invocation:

    $ git grep -e needle -- Makefile no-such-file.txt

neither Makefile or no-such-file.txt is required nor optional.  If
there are paths that match these two "patterns" among the paths in
the working tree that are known to the index, the contents of these
paths are inspected by the command.  If no paths match the patterns,
that is fine as well.

The command line parser helpfully offers to notice a pathspec
pattern that did not match any path when you do not give "--", but
that is up to the caller of match_pathspec() API to do so.  The
pathspec machinery only reports if each pathspec element matched a
path in its seen[] array, and the caller can use that information to
report which pathspec elements did not contribute to finding the set
of paths to work on.

> - The parsing is not exactly a "clean API," but I wasn't sure how to
>   make it cleaner :)

What you have in [2/3], the update to git_config_pathname(), seems
quite reasonable and something that cannot be made cleaner, to me.

> Changes in v2:
> - Only check for missing files, not empty files
> - Move a test change to the appropriate commit
> - Document optional magic in options in gitcli(1)

I agree that it is a better design not to special case an empty file
like the previous round did.  Looking better.

> This series adds support for optional filepaths in config and
> parse-options, which supports use-cases such as missing commit templates
> or blame.ignoreRevsFile values without erroring.

Yes, this is what you wanted to have at the very beginning, before
listing points you want to call attention to under "Notes" label.

Will queue.  Thanks for resurrecting the topic.