Re: [PATCH v5 2/4] docs: improve formatting in git-send-email documentation

From: Junio C Hamano <gitster@pobox.com>
Date: May 29, 2025, 18:15 UTC
Message-ID: <xmqqa56vl1uq.fsf@gitster.g>
In-Reply-To: <20250528070521.17379-3-gargaditya08@live.com>

Aditya Garg <gargaditya08@live.com> writes:

> The current documentation for git-send-email had an inconsistent use of
> "", ``, and '' for quoting. This commit improves the formatting by
> using the same style throughout the documentation.

Nice.

Show 5 quoted lines

> Also, at some places, minor grammatical errors were fixed, and some
> non existent links were removed.
>
> Finally, the cpan links of necessary perl modules have been added to
> make their installation easier.

Hmmm.

Show 7 quoted lines

>  sendemail.multiEdit::
> -	If true (default), a single editor instance will be spawned to edit
> +	If `true` (default), a single editor instance will be spawned to edit
>  	files you have to edit (patches when `--annotate` is used, and the
> -	summary when `--compose` is used). If false, files will be edited one
> +	summary when `--compose` is used). If `false`, files will be edited one
>  	after the other, spawning a new editor each time.

Looks good. "edit files you have to edit" reads somewhat funny, but the topic of this change is to correct mark-up, so it is the right thing to do to leave it as-is, at least in this step.

Show 7 quoted lines

>  sendemail.confirm::
> @@ -101,7 +101,7 @@ sendemail.signedOffCc (deprecated)::
>  
>  sendemail.smtpBatchSize::
>  	Number of messages to be sent per connection, after that a relogin
> -	will happen.  If the value is 0 or undefined, send all messages in
> +	will happen.  If the value is `0` or undefined, send all messages in

Ditto. "or undefined" will make readers wonder how they would specify such a value (i.e. 'undef' in Perl) in their configuration file, and may need rephrasing, but again not within the topic of this step.

Show 8 quoted lines

> -When `--compose` is used, git send-email will use the From, To, Cc, Bcc,
> -Subject, Reply-To, and In-Reply-To headers specified in the message. If
> -the body of the message (what you type after the headers and a blank
> -line) only contains blank (or Git: prefixed) lines, the summary won't be
> +When `--compose` is used, `git send-email` will use the 'From', 'To', 'Cc',
> +'Bcc', 'Subject', 'Reply-To', and 'In-Reply-To' headers specified in the
> +message. If the body of the message (what you type after the headers and a
> +blank line) only contains blank (or Git: prefixed) lines, the summary won't be

Shouldn't 'Git:' in "or Git: prefixed" be marked-up somehow as well?

As these mail header names are all literal parts, shouldn't ehy be marked up like `To`, `Cc`, etc.?

Show 5 quoted lines

> -	by 'c_rehash', or a single file containing one or more PEM format
> -	certificates concatenated together: see verify(1) -CAfile and
> -	-CApath for more information on these). Set it to an empty string
> +	by `c_rehash`, or a single file containing one or more PEM format
> +	certificates concatenated together). Set it to an empty string

What is this change about? grammatical errors? non existent links? cpan links? It does not look any of these.

Show 24 quoted lines

> @@ -298,18 +297,18 @@ must be used for each option.
>  	connection and authentication problems.
>  
>  --batch-size=<num>::
> -	Some email servers (e.g. smtp.163.com) limit the number emails to be
> +	Some email servers (e.g. 'smtp.163.com') limit the number of emails to be
>  	sent per session (connection) and this will lead to a failure when
>  	sending many messages.  With this option, send-email will disconnect after
> -	sending $<num> messages and wait for a few seconds (see --relogin-delay)
> -	and reconnect, to work around such a limit.  You may want to
> -	use some form of credential helper to avoid having to retype
> -	your password every time this happens.  Defaults to the
> +	sending `$<num>` messages and wait for a few seconds
> +	(see `--relogin-delay`) and reconnect, to work around such a limit.
> +	You may want to use some form of credential helper to avoid having to
> +	retype your password every time this happens.  Defaults to the
>  	`sendemail.smtpBatchSize` configuration variable.
>  
>  --relogin-delay=<int>::
> -	Waiting $<int> seconds before reconnecting to SMTP server. Used together
> -	with --batch-size option.  Defaults to the `sendemail.smtpReloginDelay`
> +	Waiting `$<int>` seconds before reconnecting to SMTP server. Used together
> +	with `--batch-size` option.  Defaults to the `sendemail.smtpReloginDelay`
>  	configuration variable.

Together with the previous hunk, "$" before the placeholder looks incorrect, but it would be preferrable to leave it alone in order to keep the patch focused on mark-up fixes alone.

As <num> and <int> are both placeholders, not something the users would literally give, neither `<num>` or `num` is appropriate mark-up for them, though. Probably "_<num>_" (without double quotes around it), if you look at Documentation/CodingGuidelines, I guess.

Show 8 quoted lines

>  Automating
> @@ -318,7 +317,7 @@ Automating
>  --no-to::
>  --no-cc::
>  --no-bcc::
> -	Clears any list of "To:", "Cc:", "Bcc:" addresses previously
> +	Clears any list of 'To:', 'Cc:', 'Bcc:' addresses previously
>  	set via config.

The same comment about mail-headers being literal applies here.

Even though the proposed log message talks about "minor grammatical errors" and "non existent links", I didn't spot any changes about them. It is very possible that they are buried in the mark-up fixes---it would make the patch much better to separate out such changes and group the changes of the exact same kind into a single patch.

I'll stop here for now; I may come back and continue from here later.

Thanks.