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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 29 May 2025, at 11:46 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 
> Aditya Garg <gargaditya08@xxxxxxxx> 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.
> 
>> 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.
> 
>> 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.
> 
>> 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.
> 
>> -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.?

I think its ok to let these remain in '', and deviate from the rules a bit.
If backticks are used, it will be a mess when rendered on the website.

> 
>> -    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.

Non existing links. Checkout the website.

> 
>> @@ -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.

Ok

> 
>> 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.

Minor grammatical errors were that of a missing full stop at the end of a line.

Missing links, look above.
> 
> I'll stop here for now; I may come back and continue from here
> later.
> 
> Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux