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