From: Junio C Hamano <gitster@pobox.com>
To: "jade via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, jade <software@lfcode.ca>
Subject: Re: [PATCH] builtin/blame: ignore nonexistent ignore files
Date: Sat, 19 Apr 2025 14:54:37 -0700 [thread overview]
Message-ID: <xmqqo6wr95r6.fsf@gitster.g> (raw)
In-Reply-To: <pull.1947.git.git.1745088194384.gitgitgadget@gmail.com> (jade via GitGitGadget's message of "Sat, 19 Apr 2025 18:43:14 +0000")
"jade via GitGitGadget" <gitgitgadget@gmail.com> writes:
> It's currently a problem to put blame.ignoreRevsFile in a global
> gitconfig, for example, to use the GitHub (and other) supported filename
> of .git-blame-ignore-revs by default if present in a repo, since the
> current implementation exits the process if it fails to open the file.
Well, that is how it is designed to be used. If the file you
specify does not exist, it is likely you made a typo, which you
would want to be told about.
I however am somewhat sympathetic to the cause. If the user had a
way to tell Git that they know what they are doing, it would make
sense in certain situations to allow the paths they specify, either
from the command line or in the configuration files, to be optional,
without triggering "no, there is no such file, perhaps you have a
typo there?" error.
Having said all that, I am not interested in the design and
implementaion presented in this patch at all, for many reasons.
- The code essentially duplicates the loop that goes over the
specified files.
- Unconditionally robbing the typo protection from existing users
who expect the command would refuse to start when they
misconfigure is not absolute no-no. We would have some way for
the user to say "I am giving this path to be used, but this is
optional. Instead of failing, just pretend I didn't specify it if
it does not exist".
- Singling out the ignorefile configuration and treating it
differently from the command line option makes things
inconsistent. Perhaps people may want to do
[alias] fooblame = blame --ignore-revs-file="foo"
[alias] barblame = blame --ignore-revs-file="bar"
instead of using a single configuration variable, and somehow
want to mark them to be "optional".
- The situation where users want to specify a pathname for a file
that is optional is not limited to blame.ignoreRevsFile. Any
codepath that takes user-specified/specifiable pathname (or blob
object name) should benefit if we had a single consistent way to
tell Git that the path is optional.
An alternative design that goes along the following lines may be
more palatable:
- The way to spell for the users to specify a path that is
optional, either as the value of a command line option or a
configuration variable, is to prefix it with ":(optional)". E.g.
[blame]
ignoreRevsFile = ":(optional).git-blame-ignore"
$ git blame --ignore-revs-file=":(optional).git-blame-ignore"
- For command line options, all commands that use parse-options API
would automatically benefit by updating parse-options.c and tweak
its handling of OPTION_FILENAME; when the specified string begins
with ":(optional)", you strip the prefix and see if the remainder
or the string names an existing file. If it does, you use the
filename as the value of that command line option; otherwise you
pretend that the option didn't even exist on the command line.
- For configuration variables, you update git_config_pathname() to
do the same.
Thanks.
next prev parent reply other threads:[~2025-04-19 21:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-19 18:43 [PATCH] builtin/blame: ignore nonexistent ignore files jade via GitGitGadget
2025-04-19 21:54 ` Junio C Hamano [this message]
2025-04-20 5:35 ` Eric Sunshine
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=xmqqo6wr95r6.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=software@lfcode.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).