Web lists-archives.com

Re: [PATCH v2] doc: format pathnames and URLs as monospace

"corentin bompard" <corentin.bompard@xxxxxxxxxxxxxxxxx> wrote:

> Updating the documentation to use monospace on URLs and pathnames because it
> makes more sense

We usually write commit message with an imperative tone, eg. "Update
documentation", not "Updating documentation". Also, the period (.) is
missing at the end of the sentence and the message is not wrapped at
72 characters (your text editor probably can do that for you; with
Emacs it's M-q or M-x auto-fill-mode RET).

But more importantly, "makes more sense" is the question here, not an
answer. The commit message is precisely here to justify why the code
after the patch makes more sense than before, and you can't argue "it
makes more sense because it makes more sense".

Among the arguments:

* It is already an established practice. For example:

  $ git grep "'[^' ]*/[^' ]*'" | wc -l
  $ git grep '`[^` ]*/[^` ]*`' | wc -l
  There are false on both sides, but after a cursory look at the
  output of both, I don't think the false positive rate is really
  higher in the second case.

  At least, this shows that the existing documentation uses inconsistent
  formatting, and that it would be good to do something about it.

* It may be debatable whether path names need to be typed in
  monospace (I wouldn't be shocked if they were using the normal
  font), but having them in italics is really unusual.

In addition to doing the actual change, you probably want to add a
mention of the rule followed in Documentation/CodingGuideline.

The patch itself looks good, except the error noted by Eric Sunshine

> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -48,7 +48,7 @@ rewriting published history.)
> Always verify that the rewritten version is correct: The original refs,
> if different from the rewritten ones, will be stored in the namespace
> -'refs/original/'.
> +`refs/original/`.
> Note that since this operation is very I/O expensive, it might
> be a good idea to redirect the temporary directory off-disk with the
> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> index 023ca95e7..9848d0d84 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -524,7 +524,7 @@ The most notable example is `HEAD`.
> [[def_remote_tracking_branch]]remote-tracking branch::
> 	A <<def_ref,ref>> that is used to follow changes from another
> 	<<def_repository,repository>>. It typically looks like
> -	'refs/remotes/foo/bar' (indicating that it tracks a branch named
> +	`refs/remotes/foo/bar` (indicating that it tracks a branch named
> 	'bar' in a remote named 'foo'), and matches the right-hand-side of
> 	a configured fetch <<def_refspec,refspec>>. A remote-tracking
> 	branch should not contain direct modifications or have local
> @@ -654,7 +654,7 @@ The most notable example is `HEAD`.
> 	The default <<def_branch,branch>> that is merged into the branch in
> 	question (or the branch in question is rebased onto). It is configured
> 	via branch.<name>.remote and branch.<name>.merge. If the upstream branch
> -	of 'A' is 'origin/B' sometimes we say "'A' is tracking 'origin/B'".
> +	of 'A' is `origin/B` sometimes we say "'A' is tracking `origin/B`".
> [[def_working_tree]]working tree::
> 	The tree of actual checked out files.  The working tree normally
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index 72daa20e7..92b1d5638 100644
> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -23,27 +23,27 @@ characters and to avoid word splitting.
>   followed by a dash and a number of commits, followed by a dash, a
>   'g', and an abbreviated object name.
> -'<refname>', e.g. 'master', 'heads/master', 'refs/heads/master'::
> +'<refname>', e.g. 'master', `heads/master`, `refs/heads/master`::

These are refnames, and you said you excluded them from the patch.

Matthieu Moy