threads / patch / 61532

patchblame: do not mention obvious default configuration values

Subject: [PATCH] blame: do not mention obvious default configuration values

## tl;dr

4 messages between May 22, 2024 and May 23, 2024. Diffs are folded; open one to read it.

replies: 3people: 4as markdown or json

Junio C Hamano· May 22, 2024, 18:44 UTC · lore

While it helps to mention what the default behaviour is when a variable is unconfigured, when there are more than true or false choices for the variable (e.g. "defaults to never, among possible choices never/local/remote/always"), or if the default behaviour is as if the unconfigured variable were set to true the knob is purely to opt out of it ("Porcelain 'git diff' refreshes the index, but diff.autoRefreshIndex can be set to false to opt out of it"), it feels like it adds unnecesary noise to say "defaults to off" for a configuration variable that is clearly a Boolean.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * I am somewhat hesitant to use a phrase that implies that there is
   some "default value" for any configuration variable.  Saying "The
   default value for the configuration variable X is Y" misleads
   readers to expect that "git config -l" would show X=Y when the
   user hasn't configured X at all.
   It is just the feature controlled by the configuration variable
   works exactly as if the configuration variable is set to that
   "default value", instead of left unconfigured.
   But that is a separate issue, and is totally outside the scope of
   this change.
 Documentation/config/blame.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
Show changes to Documentation/config/blame.txt +1 −3
diff --git a/Documentation/config/blame.txt b/Documentation/config/blame.txt
index 4d047c1790..36f965484c 100644
--- a/Documentation/config/blame.txt
+++ b/Documentation/config/blame.txt
@@ -1,6 +1,6 @@
 blame.blankBoundary::
 	Show blank commit object name for boundary commits in
-	linkgit:git-blame[1]. This option defaults to false.
+	linkgit:git-blame[1].
 
 blame.coloring::
 	This determines the coloring scheme to be applied to blame
@@ -14,11 +14,9 @@ blame.date::
 
 blame.showEmail::
 	Show the author email instead of author name in linkgit:git-blame[1].
-	This option defaults to false.
 
 blame.showRoot::
 	Do not treat root commits as boundaries in linkgit:git-blame[1].
-	This option defaults to false.
 
 blame.ignoreRevsFile::
 	Ignore revisions listed in the file, one unabbreviated object name per

-- 
2.45.1-216-g4365c6fcf9
Dragan Simic· May 22, 2024, 19:13 UTC · re: Junio C Hamano · lore

Re: [PATCH] blame: do not mention obvious default configuration values

On 2024-05-22 20:44, Junio C Hamano wrote:
Show 26 quoted lines
> diff --git a/Documentation/config/blame.txt 
> b/Documentation/config/blame.txt
> index 4d047c1790..36f965484c 100644
> --- a/Documentation/config/blame.txt
> +++ b/Documentation/config/blame.txt
> @@ -1,6 +1,6 @@
>  blame.blankBoundary::
>  	Show blank commit object name for boundary commits in
> -	linkgit:git-blame[1]. This option defaults to false.
> +	linkgit:git-blame[1].
> 
>  blame.coloring::
>  	This determines the coloring scheme to be applied to blame
> @@ -14,11 +14,9 @@ blame.date::
> 
>  blame.showEmail::
>  	Show the author email instead of author name in linkgit:git-blame[1].
> -	This option defaults to false.
> 
>  blame.showRoot::
>  	Do not treat root commits as boundaries in linkgit:git-blame[1].
> -	This option defaults to false.
> 
>  blame.ignoreRevsFile::
>  	Ignore revisions listed in the file, one unabbreviated object name 
> per

Frankly, I'd much rather see the same "If true, ..." twist [1] applied here, rather than just deleting the descriptions of the defaults. To me, such an approach eliminates any doubts, while not stressing out the actual default value.

[1] https://lore.kernel.org/git/54c318f42ce63f0575ea48f3dc1e8361@manjaro.org/

Eric Sunshine· May 22, 2024, 19:19 UTC · re: Dragan Simic · lore

Re: [PATCH] blame: do not mention obvious default configuration values

On Wed, May 22, 2024 at 3:13 PM Dragan Simic <dsimic@manjaro.org> wrote:
Show 14 quoted lines
> On 2024-05-22 20:44, Junio C Hamano wrote:
> > diff --git a/Documentation/config/blame.txt
> > b/Documentation/config/blame.txt
> > @@ -1,6 +1,6 @@
> >  blame.blankBoundary::
> >       Show blank commit object name for boundary commits in
> > -     linkgit:git-blame[1]. This option defaults to false.
> > +     linkgit:git-blame[1].
> > @@ -14,11 +14,9 @@ blame.date::
>
> Frankly, I'd much rather see the same "If true, ..." twist [1]
> applied here, rather than just deleting the descriptions of the
> defaults.  To me, such an approach eliminates any doubts, while
> not stressing out the actual default value.

For what it's worth, when reviewing patches on this list, I have multiple times asked submitters to mention the default value when adding new documentation since it saves users the trouble of having to discover the default either by experimentation or by reading the source code. So, I see removal of mention of default values as a step backward.

Jeff King· May 23, 2024, 09:08 UTC · re: Eric Sunshine · lore

Re: [PATCH] blame: do not mention obvious default configuration values

On Wed, May 22, 2024 at 03:19:58PM -0400, Eric Sunshine wrote:
Show 6 quoted lines
> For what it's worth, when reviewing patches on this list, I have
> multiple times asked submitters to mention the default value when
> adding new documentation since it saves users the trouble of having to
> discover the default either by experimentation or by reading the
> source code. So, I see removal of mention of default values as a step
> backward.

FWIW, I came here to make the same comment. ;) I'd rather the documentation err on the side of being overly descriptive.

-Peff

← back to recent threads