All of lore.kernel.org
 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 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.