All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, me@ttaylorr.com, Johannes.Schindelin@gmx.de,
	Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH] git-rebase.txt: use back-ticks consistently
Date: Tue, 28 Jun 2022 09:54:35 -0700	[thread overview]
Message-ID: <xmqqmtdwlodw.fsf@gitster.g> (raw)
In-Reply-To: <pull.1270.git.1656364861242.gitgitgadget@gmail.com> (Derrick Stolee via GitGitGadget's message of "Mon, 27 Jun 2022 21:21:01 +0000")

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

>     I can see splitting it up in a few ways, but I wanted to check to first
>     see if that was necessary. If it is, then here are the strategies I
>     considered:
>     
>      1. Focus on type of update. This would mean a change for adding
>         back-ticks on all --<option> text, adding back-ticks on all "git
>         rebase" instances, adding back-ticks on things like <upstream>, and
>         doing other types of changes like whitespace updates or "git-rebase"
>         to "git rebase".

Unless each of these steps can be verified mechanically, I do not
think it is worth it.  If I have to choose between reading (1) 700+
line patch just once in the form as-posted, or (2) three 400+ line
patches along the "type/theme", the choice is fairly simple and
obvious.

>      2. Focus on the section of the document. This would limit the diff by
>         the section size, such as OPTIONS or the discussion on the backends.

This could work, but the reviewers can choose where to stop if they
feel that a 700+ line patch is a bit too much to read through in a
single sitting themselves.

>      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 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?)


>  Documentation/git-rebase.txt | 224 +++++++++++++++++------------------
>  1 file changed, 112 insertions(+), 112 deletions(-)

> @@ -348,10 +348,10 @@ See also INCOMPATIBLE OPTIONS below.
>  	Using merging strategies to rebase (default).
>  +
>  Note that a rebase merge works by replaying each commit from the working
> -branch on top of the <upstream> branch.  Because of this, when a merge
> +branch on top of the `<upstream>` branch.  Because of this, when a merge
>  conflict happens, the side reported as 'ours' is the so-far rebased
> -series, starting with <upstream>, and 'theirs' is the working branch.  In
> -other words, the sides are swapped.
> +series, starting with `<upstream>`, and 'theirs' is the working branch.
> +In other words, the sides are swapped.
>  +
>  See also INCOMPATIBLE OPTIONS below.
>  
> @@ -360,9 +360,9 @@ See also INCOMPATIBLE OPTIONS below.
>  	Use the given merge strategy, instead of the default `ort`.
>  	This implies `--merge`.
>  +
> -Because 'git rebase' replays each commit from the working branch
> -on top of the <upstream> branch using the given strategy, using
> -the 'ours' strategy simply empties all patches from the <branch>,
> +Because `git rebase` replays each commit from the working branch
> +on top of the `<upstream>` branch using the given strategy, using
> +the `ours` strategy simply empties all patches from the `<branch>`,
>  which makes little sense.
>  +
>  See also INCOMPATIBLE OPTIONS below.
> ...
>  --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`.

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

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

> @@ -550,7 +550,7 @@ or by giving more than one `--exec`:
>  +
>  	git rebase -i --exec "cmd1" --exec "cmd2" --exec ...
>  +
> -If `--autosquash` is used, "exec" lines will not be appended for
> +If `--autosquash` is used, `exec` lines will not be appended for

Likewise.

Thanks.

  parent reply	other threads:[~2022-06-28 16:56 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-27 21:21 [PATCH] git-rebase.txt: use back-ticks consistently Derrick Stolee via GitGitGadget
2022-06-28  9:59 ` Phillip Wood
2022-06-28 19:29   ` Derrick Stolee
2022-06-28 10:22 ` Ævar Arnfjörð Bjarmason
2022-06-28 13:20   ` Rendering back-ticks in plaintext docs (was Re: [PATCH] git-rebase.txt: use back-ticks consistently) Derrick Stolee
2022-06-28 16:59     ` Junio C Hamano
2022-06-28 16:54 ` Junio C Hamano [this message]
2022-06-28 19:40   ` [PATCH] git-rebase.txt: use back-ticks consistently Derrick Stolee
2022-06-28 20:02 ` [PATCH v2] " Derrick Stolee via GitGitGadget
2022-06-28 21:49   ` Junio C Hamano
2022-06-29  9:31     ` Phillip Wood
2022-06-29 12:40       ` Derrick Stolee
2022-06-30 17:18       ` Junio C Hamano
2022-06-29 12:43     ` Derrick Stolee
2022-06-29  9:27   ` Phillip Wood
2022-06-29 12:41     ` Derrick Stolee
2022-06-29 13:21   ` [PATCH v3] " Derrick Stolee via GitGitGadget
2022-06-29 15:21     ` Phillip Wood
2022-06-30 17:25       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmtdwlodw.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=me@ttaylorr.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.