[PATCH] completion: Make sed command that generates config-list.h portable.

[PATCH] completion: Make sed command that generates config-list.h portable.

[Collin Funk]

The OpenBSD 'sed' command does not support '\n' to represent newlines in
sed expressions. This leads to the follow compiler error:

    In file included from builtin/help.c:15:
    ./config-list.h:282:18: error: use of undeclared identifier 'n'
            "gitcvs.dbUser",n       "gitcvs.dbPass",
                            ^
    1 error generated.
    gmake: *** [Makefile:2821: builtin/help.o] Error 1

We can use a variable that expands to a newline to do this portably.

This portably issue was introduced in e1b81f54da (completion: take into
account the formatting backticks for options, 2025-03-19)

Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
 generate-configlist.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/generate-configlist.sh b/generate-configlist.sh
index b06da53c89..48ec8d9812 100755
--- a/generate-configlist.sh
+++ b/generate-configlist.sh
@@ -1,5 +1,8 @@
 #!/bin/sh
 
+nl='
+'
+
 SOURCE_DIR="$1"
 OUTPUT="$2"
 
@@ -19,7 +22,7 @@ EOF
 	s/::$//;
 	s/`//g;
 	s/^.*$/	"&",/;
-	s/,  */",\n	"/g;
+	s/,  */",''"$nl"''	"/g;
 	p;};
 d' \
 	    "$SOURCE_DIR"/Documentation/*config.adoc \
-- 
2.49.0

Re: [PATCH] completion: Make sed command that generates config-list.h portable.

[Brad Smith]

On 2025-06-02 2:41 p.m., Collin Funk wrote:
> The OpenBSD 'sed' command does not support '\n' to represent newlines in
> sed expressions. This leads to the follow compiler error:
>
>      In file included from builtin/help.c:15:
>      ./config-list.h:282:18: error: use of undeclared identifier 'n'
>              "gitcvs.dbUser",n       "gitcvs.dbPass",
>                              ^
>      1 error generated.
>      gmake: *** [Makefile:2821: builtin/help.o] Error 1
>
> We can use a variable that expands to a newline to do this portably.
>
> This portably issue was introduced in e1b81f54da (completion: take into
> account the formatting backticks for options, 2025-03-19)
>
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
>   generate-configlist.sh | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/generate-configlist.sh b/generate-configlist.sh
> index b06da53c89..48ec8d9812 100755
> --- a/generate-configlist.sh
> +++ b/generate-configlist.sh
> @@ -1,5 +1,8 @@
>   #!/bin/sh
>   
> +nl='
> +'
> +
>   SOURCE_DIR="$1"
>   OUTPUT="$2"
>   
> @@ -19,7 +22,7 @@ EOF
>   	s/::$//;
>   	s/`//g;
>   	s/^.*$/	"&",/;
> -	s/,  */",\n	"/g;
> +	s/,  */",''"$nl"''	"/g;
>   	p;};
>   d' \
>   	    "$SOURCE_DIR"/Documentation/*config.adoc \


Thanks. This was the last piece I was just going to look into, but you
have provided a solution.

Re: [PATCH] completion: Make sed command that generates config-list.h portable.

[Jean-Noël AVILA]

On Monday, 2 June 2025 20:41:48 CEST Collin Funk wrote:
> The OpenBSD 'sed' command does not support '\n' to represent newlines in
> sed expressions. This leads to the follow compiler error:
> 
>     In file included from builtin/help.c:15:
>     ./config-list.h:282:18: error: use of undeclared identifier 'n'
>             "gitcvs.dbUser",n       "gitcvs.dbPass",
>                             ^
>     1 error generated.
>     gmake: *** [Makefile:2821: builtin/help.o] Error 1
> 
> We can use a variable that expands to a newline to do this portably.
> 
> This portably issue was introduced in e1b81f54da (completion: take into
> account the formatting backticks for options, 2025-03-19)
> 
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
>  generate-configlist.sh | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/generate-configlist.sh b/generate-configlist.sh
> index b06da53c89..48ec8d9812 100755
> --- a/generate-configlist.sh
> +++ b/generate-configlist.sh
> @@ -1,5 +1,8 @@
>  #!/bin/sh
> 
> +nl='
> +'
> +
>  SOURCE_DIR="$1"
>  OUTPUT="$2"
> 
> @@ -19,7 +22,7 @@ EOF
>  	s/::$//;
>  	s/`//g;
>  	s/^.*$/	"&",/;
> -	s/,  */",\n	"/g;
> +	s/,  */",''"$nl"''	"/g;
>  	p;};
>  d' \
>  	    "$SOURCE_DIR"/Documentation/*config.adoc \

Hello,

I was on this issue here: 
https://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019

Your proposed fix is interesting in that it does not spawn an additional 
process, but it does not work for me (debian sh = dash).

ᐅ diff config-list.h config-list.h.new
281d280
<       "gitcvs.dbPass",
283c282
<       "gitcvs.dbUser",
---
>       "gitcvs.dbUser","$nl"   "gitcvs.dbPass",
350,351c349
<       "http.lowSpeedLimit",
<       "http.lowSpeedTime",
---
>       "http.lowSpeedLimit","$nl"      "http.lowSpeedTime",

If you'd like to test your patch on different systems and happen to have a 
github account, you can open a PR to gitgitgadget/git . This will trigger 
builds on several targets.

Thanks,

JN

Re: [PATCH] completion: Make sed command that generates config-list.h portable.

[Collin Funk]

Jean-Noël AVILA <jn.avila@free.fr> writes:

> Hello,
>
> I was on this issue here: 
> https://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019
>
> Your proposed fix is interesting in that it does not spawn an additional 
> process, but it does not work for me (debian sh = dash).
>
> ᐅ diff config-list.h config-list.h.new
> 281d280
> <       "gitcvs.dbPass",
> 283c282
> <       "gitcvs.dbUser",
> ---
>>       "gitcvs.dbUser","$nl"   "gitcvs.dbPass",
> 350,351c349
> <       "http.lowSpeedLimit",
> <       "http.lowSpeedTime",
> ---
>>       "http.lowSpeedLimit","$nl"      "http.lowSpeedTime",
>
> If you'd like to test your patch on different systems and happen to have a 
> github account, you can open a PR to gitgitgadget/git . This will trigger 
> builds on several targets.

Thank you very much for checking! On my system /bin/sh is linked to
/bin/bash, so I did not notice.

I will send a V2 that works with the dash packaged by Fedora 42. It
produces the same output as bash and works on OpenBSD.

Collin

[PATCH v2] completion: Make sed command that generates config-list.h portable.

[Collin Funk]

The OpenBSD 'sed' command does not support '\n' to represent newlines in
sed expressions. This leads to the follow compiler error:

    In file included from builtin/help.c:15:
    ./config-list.h:282:18: error: use of undeclared identifier 'n'
            "gitcvs.dbUser",n       "gitcvs.dbPass",
                            ^
    1 error generated.
    gmake: *** [Makefile:2821: builtin/help.o] Error 1

We can use a backslash followed by a newline to fix this.

This portably issue was introduced in e1b81f54da (completion: take into
account the formatting backticks for options, 2025-03-19)

Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
 generate-configlist.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/generate-configlist.sh b/generate-configlist.sh
index b06da53c89..e1f9e99488 100755
--- a/generate-configlist.sh
+++ b/generate-configlist.sh
@@ -19,7 +19,8 @@ EOF
 	s/::$//;
 	s/`//g;
 	s/^.*$/	"&",/;
-	s/,  */",\n	"/g;
+	s/,  */",\
+	"/g;
 	p;};
 d' \
 	    "$SOURCE_DIR"/Documentation/*config.adoc \
-- 
2.49.0

Re: [PATCH v2] completion: Make sed command that generates config-list.h portable.

[Jean-Noël AVILA]

On Monday, 2 June 2025 21:26:47 CEST Collin Funk wrote:
> The OpenBSD 'sed' command does not support '\n' to represent newlines in
> sed expressions. This leads to the follow compiler error:
> 
>     In file included from builtin/help.c:15:
>     ./config-list.h:282:18: error: use of undeclared identifier 'n'
>             "gitcvs.dbUser",n       "gitcvs.dbPass",
>                             ^
>     1 error generated.
>     gmake: *** [Makefile:2821: builtin/help.o] Error 1
> 
> We can use a backslash followed by a newline to fix this.
> 
> This portably issue was introduced in e1b81f54da (completion: take into
> account the formatting backticks for options, 2025-03-19)
> 
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
>  generate-configlist.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/generate-configlist.sh b/generate-configlist.sh
> index b06da53c89..e1f9e99488 100755
> --- a/generate-configlist.sh
> +++ b/generate-configlist.sh
> @@ -19,7 +19,8 @@ EOF
>  	s/::$//;
>  	s/`//g;
>  	s/^.*$/	"&",/;
> -	s/,  */",\n	"/g;
> +	s/,  */",\
> +	"/g;
>  	p;};
>  d' \
>  	    "$SOURCE_DIR"/Documentation/*config.adoc \

FYI I pushed your patch to 
https://github.com/gitgitgadget/git/pull/1930/

It seems to be passing, although it's not very readable.

Your commit message has some issues:

 * upper case in "Make": prefixed commits message must be lower case
 * to be correct, the bug was already there at the first introduction of the 
generate-configlist.sh script (3ac68a9). The '\n' was there, and the generated 
.h file had two wrong strings such as
"gitcvs.dbUserngitcvs.dbPass" . My patch only put it in light by breaking the 
build.

Maybe an alternative way of fixing the issue is to just rework the 
documentation on the two spots where a comma is used and put each config 
variable on its own line.

What do you think?

JN

Re: [PATCH v2] completion: Make sed command that generates config-list.h portable.

[Collin Funk]

Jean-Noël AVILA <jn.avila@free.fr> writes:

> Your commit message has some issues:
>
>  * upper case in "Make": prefixed commits message must be lower case
>  * to be correct, the bug was already there at the first introduction of the 
> generate-configlist.sh script (3ac68a9). The '\n' was there, and the generated 
> .h file had two wrong strings such as
> "gitcvs.dbUserngitcvs.dbPass" . My patch only put it in light by breaking the 
> build.

Thanks, I will keep that in mind for V3.

> Maybe an alternative way of fixing the issue is to just rework the 
> documentation on the two spots where a comma is used and put each config 
> variable on its own line.
>
> What do you think?

Regarding readability, it is not any worse than it was originally. But
maybe that is because you are much better at sed than me. :)

But we could put configurations on seperate lines like so:

diff --git a/Documentation/config/gitcvs.adoc b/Documentation/config/gitcvs.adoc
index 02da427fd9..31d7be3992 100644
--- a/Documentation/config/gitcvs.adoc
+++ b/Documentation/config/gitcvs.adoc
@@ -47,7 +47,8 @@ gitcvs.dbDriver::
        May not contain double colons (`:`). Default: 'SQLite'.
        See linkgit:git-cvsserver[1].
 
-gitcvs.dbUser, gitcvs.dbPass::
+gitcvs.dbUser::
+gitcvs.dbPass::
        Database user and password. Only useful if setting `gitcvs.dbDriver`,
        since SQLite has no concept of database users and/or passwords.
        'gitcvs.dbUser' supports variable substitution (see
diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc
index 67393282fa..9da5c298cc 100644
--- a/Documentation/config/http.adoc
+++ b/Documentation/config/http.adoc
@@ -289,7 +289,8 @@ for most push problems, but can increase memory consumption
 significantly since the entire buffer is allocated even for small
 pushes.
 
-http.lowSpeedLimit, http.lowSpeedTime::
+http.lowSpeedLimit::
+http.lowSpeedTime::
        If the HTTP transfer speed, in bytes per second, is less than
        'http.lowSpeedLimit' for longer than 'http.lowSpeedTime' seconds,
        the transfer is aborted.

This is similar to how options are documented, for example:

    $ head Documentation/pretty-options.adoc 
    --pretty[=<format>]::
    --format=<format>::
    
    	Pretty-print the contents of the commit logs in a given format,
    	where '<format>' can be one of 'oneline', 'short', 'medium',
    	'full', 'fuller', 'reference', 'email', 'raw', 'format:<string>'
    	and 'tformat:<string>'.  When '<format>' is none of the above,
    	and has '%placeholder' in it, it acts as if
    	'--pretty=tformat:<format>' were given.

Then go back to the simpler sed expression before your most recent
commit. I will wait for others thoughts before posting v3.

Thanks,
Collin

Re: [PATCH v2] completion: Make sed command that generates config-list.h portable.

[Jacob Keller]

On 6/2/2025 1:08 PM, Collin Funk wrote:
> Jean-Noël AVILA <jn.avila@free.fr> writes:
> 
>> Your commit message has some issues:
>>
>>  * upper case in "Make": prefixed commits message must be lower case
>>  * to be correct, the bug was already there at the first introduction of the 
>> generate-configlist.sh script (3ac68a9). The '\n' was there, and the generated 
>> .h file had two wrong strings such as
>> "gitcvs.dbUserngitcvs.dbPass" . My patch only put it in light by breaking the 
>> build.
> 
> Thanks, I will keep that in mind for V3.
> 
>> Maybe an alternative way of fixing the issue is to just rework the 
>> documentation on the two spots where a comma is used and put each config 
>> variable on its own line.
>>
>> What do you think?
> 
> Regarding readability, it is not any worse than it was originally. But
> maybe that is because you are much better at sed than me. :)
> 
> But we could put configurations on seperate lines like so:
> 
> diff --git a/Documentation/config/gitcvs.adoc b/Documentation/config/gitcvs.adoc
> index 02da427fd9..31d7be3992 100644
> --- a/Documentation/config/gitcvs.adoc
> +++ b/Documentation/config/gitcvs.adoc
> @@ -47,7 +47,8 @@ gitcvs.dbDriver::
>         May not contain double colons (`:`). Default: 'SQLite'.
>         See linkgit:git-cvsserver[1].
>  
> -gitcvs.dbUser, gitcvs.dbPass::
> +gitcvs.dbUser::
> +gitcvs.dbPass::
>         Database user and password. Only useful if setting `gitcvs.dbDriver`,
>         since SQLite has no concept of database users and/or passwords.
>         'gitcvs.dbUser' supports variable substitution (see
> diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc
> index 67393282fa..9da5c298cc 100644
> --- a/Documentation/config/http.adoc
> +++ b/Documentation/config/http.adoc
> @@ -289,7 +289,8 @@ for most push problems, but can increase memory consumption
>  significantly since the entire buffer is allocated even for small
>  pushes.
>  
> -http.lowSpeedLimit, http.lowSpeedTime::
> +http.lowSpeedLimit::
> +http.lowSpeedTime::
>         If the HTTP transfer speed, in bytes per second, is less than
>         'http.lowSpeedLimit' for longer than 'http.lowSpeedTime' seconds,
>         the transfer is aborted.
> 
> This is similar to how options are documented, for example:
> 
>     $ head Documentation/pretty-options.adoc 
>     --pretty[=<format>]::
>     --format=<format>::
>     
>     	Pretty-print the contents of the commit logs in a given format,
>     	where '<format>' can be one of 'oneline', 'short', 'medium',
>     	'full', 'fuller', 'reference', 'email', 'raw', 'format:<string>'
>     	and 'tformat:<string>'.  When '<format>' is none of the above,
>     	and has '%placeholder' in it, it acts as if
>     	'--pretty=tformat:<format>' were given.
> 
> Then go back to the simpler sed expression before your most recent
> commit. I will wait for others thoughts before posting v3.
> 

I like this approach.

> Thanks,
> Collin
> 

[PATCH v3] completion: make sed command that generates config-list.h portable.

[Collin Funk]

The OpenBSD 'sed' command does not support '\n' to represent newlines in
sed expressions. This leads to the follow compiler error:

    In file included from builtin/help.c:15:
    ./config-list.h:282:18: error: use of undeclared identifier 'n'
            "gitcvs.dbUser",n       "gitcvs.dbPass",
                            ^
    1 error generated.
    gmake: *** [Makefile:2821: builtin/help.o] Error 1

We can fix this by documenting related configuration variables
one-per-line instead of listing them separated by commas. This allows us
to remove the unportable part of the sed expression in
generate-configlist.sh.

Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
 Documentation/config/gitcvs.adoc | 3 ++-
 Documentation/config/http.adoc   | 3 ++-
 generate-configlist.sh           | 1 -
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/gitcvs.adoc b/Documentation/config/gitcvs.adoc
index 02da427fd9..31d7be3992 100644
--- a/Documentation/config/gitcvs.adoc
+++ b/Documentation/config/gitcvs.adoc
@@ -47,7 +47,8 @@ gitcvs.dbDriver::
 	May not contain double colons (`:`). Default: 'SQLite'.
 	See linkgit:git-cvsserver[1].
 
-gitcvs.dbUser, gitcvs.dbPass::
+gitcvs.dbUser::
+gitcvs.dbPass::
 	Database user and password. Only useful if setting `gitcvs.dbDriver`,
 	since SQLite has no concept of database users and/or passwords.
 	'gitcvs.dbUser' supports variable substitution (see
diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc
index 67393282fa..9da5c298cc 100644
--- a/Documentation/config/http.adoc
+++ b/Documentation/config/http.adoc
@@ -289,7 +289,8 @@ for most push problems, but can increase memory consumption
 significantly since the entire buffer is allocated even for small
 pushes.
 
-http.lowSpeedLimit, http.lowSpeedTime::
+http.lowSpeedLimit::
+http.lowSpeedTime::
 	If the HTTP transfer speed, in bytes per second, is less than
 	'http.lowSpeedLimit' for longer than 'http.lowSpeedTime' seconds,
 	the transfer is aborted.
diff --git a/generate-configlist.sh b/generate-configlist.sh
index b06da53c89..9d2ad6165d 100755
--- a/generate-configlist.sh
+++ b/generate-configlist.sh
@@ -19,7 +19,6 @@ EOF
 	s/::$//;
 	s/`//g;
 	s/^.*$/	"&",/;
-	s/,  */",\n	"/g;
 	p;};
 d' \
 	    "$SOURCE_DIR"/Documentation/*config.adoc \
-- 
2.49.0

Re: [PATCH v2] completion: Make sed command that generates config-list.h portable.

[Collin Funk]

Hi Jacob,

Jacob Keller <jacob.e.keller@intel.com> writes:

>> Then go back to the simpler sed expression before your most recent
>> commit. I will wait for others thoughts before posting v3.
>> 
>
> I like this approach.

Thanks for checking.

Although, I now realize my explination was slightly incorrect. Some of
the referenced commit is still needed for formatting, but we can remove
part of the expression that causes the problem.

I posted V3 so others can check.

Collin

RE: [PATCH v3] completion: make sed command that generates config-list.h portable.

[Keller, Jacob E]

> -----Original Message-----
> From: Collin Funk <collin.funk1@gmail.com>
> Sent: Monday, June 2, 2025 3:32 PM
> To: git@vger.kernel.org
> Cc: jn.avila@free.fr; Keller, Jacob E <jacob.e.keller@intel.com>; Collin Funk
> <collin.funk1@gmail.com>; Corentin Garcia <corenting@gmail.com>; Junio C
> Hamano <gitster@pobox.com>
> Subject: [PATCH v3] completion: make sed command that generates config-list.h
> portable.
> 
> The OpenBSD 'sed' command does not support '\n' to represent newlines in
> sed expressions. This leads to the follow compiler error:
> 
>     In file included from builtin/help.c:15:
>     ./config-list.h:282:18: error: use of undeclared identifier 'n'
>             "gitcvs.dbUser",n       "gitcvs.dbPass",
>                             ^
>     1 error generated.
>     gmake: *** [Makefile:2821: builtin/help.o] Error 1
> 
> We can fix this by documenting related configuration variables
> one-per-line instead of listing them separated by commas. This allows us
> to remove the unportable part of the sed expression in
> generate-configlist.sh.
> 
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH v3] completion: make sed command that generates config-list.h portable.

[Junio C Hamano]

"Keller, Jacob E" <jacob.e.keller@intel.com> writes:

>> -----Original Message-----
>> From: Collin Funk <collin.funk1@gmail.com>
>> Sent: Monday, June 2, 2025 3:32 PM
>> To: git@vger.kernel.org
>> Cc: jn.avila@free.fr; Keller, Jacob E <jacob.e.keller@intel.com>; Collin Funk
>> <collin.funk1@gmail.com>; Corentin Garcia <corenting@gmail.com>; Junio C
>> Hamano <gitster@pobox.com>
>> Subject: [PATCH v3] completion: make sed command that generates config-list.h
>> portable.
>> 
>> The OpenBSD 'sed' command does not support '\n' to represent newlines in
>> sed expressions. This leads to the follow compiler error:
>> 
>>     In file included from builtin/help.c:15:
>>     ./config-list.h:282:18: error: use of undeclared identifier 'n'
>>             "gitcvs.dbUser",n       "gitcvs.dbPass",
>>                             ^
>>     1 error generated.
>>     gmake: *** [Makefile:2821: builtin/help.o] Error 1
>> 
>> We can fix this by documenting related configuration variables
>> one-per-line instead of listing them separated by commas. This allows us
>> to remove the unportable part of the sed expression in
>> generate-configlist.sh.
>> 
>> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
>> ---
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks, all.

Re: [PATCH v2] completion: Make sed command that generates config-list.h portable.

[Junio C Hamano]

Jacob Keller <jacob.e.keller@intel.com> writes:

>> But we could put configurations on seperate lines like so:
>> 
>> ... 
>> -gitcvs.dbUser, gitcvs.dbPass::
>> +gitcvs.dbUser::
>> +gitcvs.dbPass::
>> ...
>> Then go back to the simpler sed expression before your most recent
>> commit. I will wait for others thoughts before posting v3.
>
> I like this approach.

As long as we warn our documenters that comma separated entries are
forbidden in our documentation (even though AsciiDoc and Asciidoctor
may allow them), it would be the simplest.  The question is where.

Perhaps the tail end of the CodingGuidelines document where we
already have write-up for markups?

Thanks.

[PATCH] CodingGuidelines: document formatting required by generate-configlist.sh.

[Collin Funk]

Document that related `git config` variables should be placed
one-per-line instead of separated by commas.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
 Documentation/CodingGuidelines | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index c1046abfb7..3a7e644acf 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -877,6 +877,15 @@ Characters are also surrounded by underscores:
 As a side effect, backquoted placeholders are correctly typeset, but
 this style is not recommended.
 
+ When documenting multiple related `git config` variables, place them on
+ a separate line instead of separating them by commas. For example:
+   core.var1::
+   core.var2::
+  	This is a description of 'core.var1' and 'core.var2'.
+
+This format is required for the `generate-configlist.sh` script to
+properly generate "config-list.h".
+
 Synopsis Syntax
 
  The synopsis (a paragraph with [synopsis] attribute) is automatically
-- 
2.49.0

Re: [PATCH] CodingGuidelines: document formatting required by generate-configlist.sh.

[Junio C Hamano]

Collin Funk <collin.funk1@gmail.com> writes:

> + When documenting multiple related `git config` variables, place them on
> + a separate line instead of separating them by commas. For example:
> +   core.var1::
> +   core.var2::
> +  	This is a description of 'core.var1' and 'core.var2'.

As `core.varN` in the above example are all what the end-user would
give literally, just like `git config` command name in the first
sentence, they should be marked up as literal strings, i.e.

    ... For example, do not write this:

    `core.var1`, `core.var2`::
	Description common to `core.var1` and `core.var2`.
	
    Instead write this:

    `core.var1`::
    `core.var2`::
	Description common to `core.var1` and `core.var2`.

> +This format is required for the `generate-configlist.sh` script to
> +properly generate "config-list.h".

It is not wrong per-se, but this tempts people to "fix" the
generate-configlist.sh script so that it can grok the comma
separated list "again".  And when that fix is done and reviewed
carelessly, we'd again break some implementations of sed the same
way and we will come back full circle ;-)


When we standardized writing negatable options this way

    `--option`::
    `--no-option`::
	Description of `--option` that can be turned off with
	`--no-option`.

instead of

    `--[no-]option`::
	Description of `--option` that can be turned off with
	`--no-option`.

we explained that the reason why we want to do so is because it is
easier to "grep".  Does this "do not comma-list variables, but list
them one per line" also give us better greppability, and if so we
want to explain that way, perhaps?

    $ git grep '`core\.var1`::' Documentation/config/

Thanks.

Re: [PATCH] CodingGuidelines: document formatting required by generate-configlist.sh.

[Collin Funk]

Hi Junio,

Junio C Hamano <gitster@pobox.com> writes:

> Collin Funk <collin.funk1@gmail.com> writes:
>
>> + When documenting multiple related `git config` variables, place them on
>> + a separate line instead of separating them by commas. For example:
>> +   core.var1::
>> +   core.var2::
>> +  	This is a description of 'core.var1' and 'core.var2'.
>
> As `core.varN` in the above example are all what the end-user would
> give literally, just like `git config` command name in the first
> sentence, they should be marked up as literal strings, i.e.
>
>     ... For example, do not write this:
>
>     `core.var1`, `core.var2`::
> 	Description common to `core.var1` and `core.var2`.
> 	
>     Instead write this:
>
>     `core.var1`::
>     `core.var2`::
> 	Description common to `core.var1` and `core.var2`.

This markup is different than what is used in
Documentation/config/*.adoc though. Here is just one example:

    $ head -n 3 Documentation/config/core.adoc 
    core.fileMode::
    	Tells Git if the executable bit of files in the working tree
    	is to be honored.

That was my reasoning for writing it how I did in the patch. Are you
saying that all of these should be changed? I do not have any experience
with AsciiDoc so I am not sure if that is correct.

>> +This format is required for the `generate-configlist.sh` script to
>> +properly generate "config-list.h".
>
> It is not wrong per-se, but this tempts people to "fix" the
> generate-configlist.sh script so that it can grok the comma
> separated list "again".  And when that fix is done and reviewed
> carelessly, we'd again break some implementations of sed the same
> way and we will come back full circle ;-)
> [...]
> we explained that the reason why we want to do so is because it is
> easier to "grep".  Does this "do not comma-list variables, but list
> them one per line" also give us better greppability, and if so we
> want to explain that way, perhaps?
>
>     $ git grep '`core\.var1`::' Documentation/config/

Yes, that is a good side affect of the change that can be documented.

I will send V2 after clarification on the other point.

Collin

Re: [PATCH] CodingGuidelines: document formatting required by generate-configlist.sh.

[Junio C Hamano]

Collin Funk <collin.funk1@gmail.com> writes:

>>     `core.var1`::
>>     `core.var2`::
>> 	Description common to `core.var1` and `core.var2`.
>
> This markup is different than what is used in
> Documentation/config/*.adoc though.

We are updating them gradually while avoiding collisions with
patches that do other "real" work; see many recent patches to
Documentation/config/ area by Jean-Noël Avila for more, e.g.
d30c5cc4 (doc: convert git-mergetool options to new synopsis style,
2025-05-25).

Thanks.

[PATCH v2] CodingGuidelines: document formatting of similar config variables.

[Collin Funk]

Document that related `git config` variables should be placed
one-per-line instead of separated by commas.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
 Documentation/CodingGuidelines | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index c1046abfb7..3dd339f802 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -877,6 +877,17 @@ Characters are also surrounded by underscores:
 As a side effect, backquoted placeholders are correctly typeset, but
 this style is not recommended.
 
+ When documenting multiple related `git config` variables, place them on
+ a separate line instead of separating them by commas.  For example, do
+ not write this:
+   `core.var1`, `core.var2`::
+  	Description common to `core.var1` and `core.var2`.
+
+Instead write this:
+   `core.var1`::
+   `core.var2`::
+  	Description common to `core.var1` and `core.var2`.
+
 Synopsis Syntax
 
  The synopsis (a paragraph with [synopsis] attribute) is automatically
-- 
2.49.0

Re: [PATCH] CodingGuidelines: document formatting required by generate-configlist.sh.

[Collin Funk]

Junio C Hamano <gitster@pobox.com> writes:

> We are updating them gradually while avoiding collisions with
> patches that do other "real" work; see many recent patches to
> Documentation/config/ area by Jean-Noël Avila for more, e.g.
> d30c5cc4 (doc: convert git-mergetool options to new synopsis style,
> 2025-05-25).

Ah, okay. I must have just looked at files that have not been touched in
some time.

I sent V2 which uses the example you sent [1]. Feel free to add yourself
as 'Co-authored-by:'.

Thanks,
Collin

[1] https://lore.kernel.org/git/802402a288f0976765f1ba1c82d14c2289c8cf72.1748990700.git.collin.funk1@gmail.com/T/#u

Re: [PATCH v2] CodingGuidelines: document formatting of similar config variables.

[Junio C Hamano]

Collin Funk <collin.funk1@gmail.com> writes:

> Document that related `git config` variables should be placed
> one-per-line instead of separated by commas.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
>  Documentation/CodingGuidelines | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Looks good.  Thanks.

> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index c1046abfb7..3dd339f802 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -877,6 +877,17 @@ Characters are also surrounded by underscores:
>  As a side effect, backquoted placeholders are correctly typeset, but
>  this style is not recommended.
>  
> + When documenting multiple related `git config` variables, place them on
> + a separate line instead of separating them by commas.  For example, do
> + not write this:
> +   `core.var1`, `core.var2`::
> +  	Description common to `core.var1` and `core.var2`.
> +
> +Instead write this:
> +   `core.var1`::
> +   `core.var2`::
> +  	Description common to `core.var1` and `core.var2`.
> +
>  Synopsis Syntax
>  
>   The synopsis (a paragraph with [synopsis] attribute) is automatically