git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Dragan Simic <dsimic@manjaro.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 4/5] grep: introduce new config option to include untracked files
Date: Tue, 19 Mar 2024 07:32:41 -0700	[thread overview]
Message-ID: <xmqqil1is35i.fsf@gitster.g> (raw)
In-Reply-To: <923c5f84c44a195d6ba08503e28851af@manjaro.org> (Dragan Simic's message of "Tue, 19 Mar 2024 06:47:48 +0100")

Dragan Simic <dsimic@manjaro.org> writes:

>>> +	if (use_index && !cached)
>>> +		git_config_get_bool("grep.includeuntracked", &untracked);
>> Can this ever return an error?  E.g.
>> 	[grep] includeuntracked = "not really"
>
> After a brief inspection of the code in cache.c, git_config_get_bool()
> always returns either 0 or 1, so we should be fine.  Thus, any
> strangeness in a configuration file would end up not enabling
> this option.

If that were the case, then it is not "fine".

When the user triggered an operation which *requires* us to parse
and interpret the meaning of an entry in their configuration file
correctly in order to carry it out, and if that entry has a value
that we do not consider valid, we should notice and complain, before
saying "Nah, this I do not understand, so I'll do one of the two
things I would have done if the value were understandable and would
not tell the user which one I did".

What makes it fine int his case is that git_config_get_bool() dies
when the given value is not a Boolean ;-).  The returned value from
the function can be used to tell if the variable does not exist and
the caller should decide to stuff some default value to &untracked
but in this case you do not need to.

  reply	other threads:[~2024-03-19 14:32 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-18 17:03 [PATCH 0/5] New config option for git-grep to include untracked files Dragan Simic
2024-03-18 17:03 ` [PATCH 1/5] grep: perform some minor code and comment cleanups Dragan Simic
2024-03-18 19:59   ` Eric Sunshine
2024-03-18 22:03     ` Dragan Simic
2024-03-19  0:32     ` Junio C Hamano
2024-03-19  5:33       ` Dragan Simic
2024-03-18 17:03 ` [PATCH 2/5] grep docs: describe --recurse-submodules further and improve formatting a bit Dragan Simic
2024-03-18 20:02   ` Eric Sunshine
2024-03-18 22:14     ` Dragan Simic
2024-03-18 17:03 ` [PATCH 3/5] grep docs: describe --no-index further Dragan Simic
2024-03-19  0:55   ` Junio C Hamano
2024-03-19  5:37     ` Dragan Simic
2024-03-18 17:03 ` [PATCH 4/5] grep: introduce new config option to include untracked files Dragan Simic
2024-03-19  0:58   ` Junio C Hamano
2024-03-19  5:47     ` Dragan Simic
2024-03-19 14:32       ` Junio C Hamano [this message]
2024-03-19 14:52         ` Dragan Simic
2024-03-18 17:03 ` [PATCH 5/5] grep docs: describe " Dragan Simic
2024-03-19  0:21 ` [PATCH 0/5] New config option for git-grep " Junio C Hamano
2024-03-19  5:09   ` Dragan Simic
2024-03-19 17:43     ` Junio C Hamano
2024-03-19 17:48       ` Dragan Simic

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=xmqqil1is35i.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    /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).