Re: [PATCH] git-rebase.txt: use back-ticks consistently

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

 



On 6/28/2022 12:54 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> 
>>      3. Focus on the edits that most-recently edited these lines. Doing some
>>         scripting, I was able to construct this date-sorted list of previous
>>         edits (by diffing the git blame output before and after this
>>         change). The most-recent changes before this are:
>>     
>>     2005-08-26: 52a22d1e726 ([PATCH] Subject: [PATCH] Add some documentation., 2005-08-26)
>> ...
>>     2022-04-20: 9e5ebe9668a (rebase: use correct base for --keep-base when a branch is given, 2022-04-20)
> 
> That is a new concept ;-) 
> 
> It is an interesting exercise to see which previous changes had
> these mark-up mistakes, but it is not immediately obvious to me how
> we can take advantage of the information.

I just wanted to see how much of these edits were due to "really old
documentation from when we had different habits" and how many were
related to recent updates, so I should give more pause. I think the
discussion about the "apply" versus "merge" backends should have
given me more pause because those sections were written recently and
are more likely to have correct punctuation.
 
>>     I look forward to feedback on how to do this better (if it is indeed a
>>     good idea to do in the first place).
> 
> Correcting mark-up to result in an easier-to-read documentation is a
> good idea, of course.  I wonder if we can also help the developers
> mark-up correctly in their first attempt (e.g. do we have clear and
> concise guidelines that are well publicized?)

I'm not sure we have them well publicized. It's worth thinking about.

Hopefully creating a more standard use within at least one (long) file
will help establish some patterns organically.

>>  --strategy-option=<strategy-option>::
>>  	Pass the <strategy-option> through to the merge strategy.
>>  	This implies `--merge` and, if no strategy has been
>> -	specified, `-s ort`.  Note the reversal of 'ours' and
>> -	'theirs' as noted above for the `-m` option.
>> +	specified, `-s ort`.  Note the reversal of `ours` and
>> +	`theirs` as noted above for the `-m` option.
> 
> These references to "ours" and "theirs" is what we called out
> earlier in the "swapped" description (hunk -348,10), i.e.
> 
> 	when a merge conflict happens, the side reported as 'ours'
> 	is the so-far rebased series ... and 'theirs' is the working
> 	branch.
> 
> which the patch left in 'emphasis' not `verbatim`.  I think this
> section should do the same.
> 
> The 'ours' (but not 'theirs' because there is no such thing) that is
> explained as useless as a strategy in the intervening paragraph
> (hunk -360,9) refers to the name of a strategy, and it is correct to
> mark it as `verbatim`.
Thanks!

>>  --ignore-whitespace::
>>  	Ignore whitespace differences when trying to reconcile
>> -differences. Currently, each backend implements an approximation of
>> -this behavior:
>> +	differences. Currently, each backend implements an approximation of
>> +	this behavior:
>>  +
>> -apply backend: When applying a patch, ignore changes in whitespace in
>> +'apply backend:' When applying a patch, ignore changes in whitespace in
>> ...
>> -merge backend: Treat lines with only whitespace changes as unchanged
>> +'merge backend:' Treat lines with only whitespace changes as unchanged
> 
> It somehow looks curious (at the source level---I haven't seen the
> formatted HTML output) to have the punctuation colon as part of the
> phrase marked up.  I wonder if these were meant to be more like so:
> 
> 	apply backend;;
> 		When applying a patch, ...
> 
> 	merge backend;;
> 		Treat lines with ...

This appears to work well. The doc-diff shows this:

-           apply backend: When applying a patch, ignore changes in whitespace
-           in context lines. Unfortunately, this means that if the "old" lines
-           being replaced by the patch differ only in whitespace from the
-           existing file, you will get a merge conflict instead of a
-           successful patch application.
+           apply backend:
+               When applying a patch, ignore changes in whitespace in context
+               lines. Unfortunately, this means that if the "old" lines being
+               replaced by the patch differ only in whitespace from the
+               existing file, you will get a merge conflict instead of a
+               successful patch application.
 
-           merge backend: Treat lines with only whitespace changes as
-           unchanged when merging. Unfortunately, this means that any patch
-           hunks that were intended to modify whitespace and nothing else will
-           be dropped, even if the other side had no changes that conflicted.
+           merge backend:
+               Treat lines with only whitespace changes as unchanged when
+               merging. Unfortunately, this means that any patch hunks that
+               were intended to modify whitespace and nothing else will be
+               dropped, even if the other side had no changes that conflicted.

>> @@ -536,8 +536,8 @@ See also REBASING MERGES and INCOMPATIBLE OPTIONS below.
>>  
>>  -x <cmd>::
>>  --exec <cmd>::
>> -	Append "exec <cmd>" after each line creating a commit in the
>> -	final history. <cmd> will be interpreted as one or more shell
>> +	Append `exec <cmd>` after each line creating a commit in the
>> +	final history. `<cmd>` will be interpreted as one or more shell
>>  	commands. Any command that fails will interrupt the rebase,
>>  	with exit code 1.
> 
> As noticed by others, "git help -m rebase" is somewhat harmed with
> this change when rendered to plain text without any attributes.  The
> roff output actually is
> 
>     .RS 4
>     Append
>     \fBexec <cmd>\fR
>     after each line creating a commit in the final history\&.
> 
> and even on plain text tty, "exec <cmd>" part is now shown in bold
> (as opposed to be in plain text inside double quotes, which was how
> the original got rendered).  So I think this change is an
> improvement.

Thanks for digging into the details here. It would be interesting if
doc-diff could make this clearer, but that's a project for another time.

Thanks,
-Stolee



[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