All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Noah Pendleton <noah.pendleton@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/1] blame: Skip missing ignore-revs file
Date: Mon, 09 Aug 2021 08:47:58 -0700	[thread overview]
Message-ID: <xmqq5ywehb69.fsf@gitster.g> (raw)
In-Reply-To: <CADm0i39LV91kochHSGVHovaTbDOd0COrQPXHD3x8rEj-1Y+eMA@mail.gmail.com> (Noah Pendleton's message of "Sun, 8 Aug 2021 14:21:44 -0400")

Noah Pendleton <noah.pendleton@gmail.com> writes:

> Very good point- I see about 21 call sites for `git_config_pathname`,
> plus a few others (`git_config_get_pathname`) that bottom out in the
> same function. I could see the utility of optional paths for some of
> them: for example, `commit.template`, `core.excludesfile`. Some of the
> others seem a little more ambiguous, eg `http.sslcert` probably wants
> to always fail in case of missing file.

Thanks for already doing initial surveillance.  Very useful.

> There seems to be a mix of fail-hard on invalid paths, printing a
> warning message and skipping, and silently ignoring.
>
> Hard for me to predict what the least confusing behavior is around
> path configuration values, though, so maybe adding support for the
> `:(optional)` (and maybe additionally a `:(required)`) tag across the
> board to pathname configs is the right move.

I originally hoped only ":(optional)" would be necessary, but to
keep the continuity in behaviour for those currently that do not die
upon seeing a missing file, we probably should treat an unadorned
value as asking for the "traditional" behaviour, at least in the
shorter term, and allow those users who want to detect typos to
tighten the rule using ":(required)".  I dunno.

> That patch might be beyond what I'm capable of, though I'm happy to
> put up a draft that applies it to the original `ignoreRevsFile` case
> as a starting point.

Thanks for an offer.  We are not in a hurry (especially during the
pre-release feature freeze), and hopefully this discussion would
pique other developers' interest to nudge them to help ;-)

  reply	other threads:[~2021-08-09 15:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-07 20:27 [PATCH 0/1] blame: Skip missing ignore-revs file Noah Pendleton
2021-08-07 20:58 ` Junio C Hamano
2021-08-07 21:34   ` Noah Pendleton
2021-08-08  5:43     ` Junio C Hamano
2021-08-08 17:50       ` Junio C Hamano
2021-08-08 18:21         ` Noah Pendleton
2021-08-09 15:47           ` Junio C Hamano [this message]
2022-03-04  9:51   ` Thranur Andul
2021-08-08 17:48 ` [PATCH v2] blame: add config `blame.ignoreRevsFileIsOptional` Noah Pendleton
2021-08-07 20:29 [PATCH 0/1] blame: Skip missing ignore-revs file Noah Pendleton

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=xmqq5ywehb69.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=noah.pendleton@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.