git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/4] documentation: trivial style cleanups

From
Junio C Hamano <gitster@pobox.com>
Date
May 9, 2013, 23:09 UTC
Message-ID
<7vehdf7nts.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1368062218-22440-2-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> White-spaces, missing braces, standardize --[no-]foo.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

This is uncomfortably big at this phase of the release cycle, but thanks anyway.

Because I didn't want to review this patch only to spot silly formatting mistakes that may break the documentation build (which I didn't find any), I looked the entire files the patch touches, and many of the comments below ended up being suggestions for a follow-up work for people who would want to pick low-hanging fruits.

Hint, hint.

There however are a few things that should have been in this patch, though. I'll attach what I'd queue as "SQUASH???" on top of this patch at the bottom.

Show 29 quoted lines
> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index 19d57a8..5bbe7b6 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -9,12 +9,12 @@ git-am - Apply a series of patches from a mailbox
>  SYNOPSIS
>  --------
>  [verse]
> -'git am' [--signoff] [--keep] [--keep-cr | --no-keep-cr] [--utf8 | --no-utf8]
> +'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
>  	 [--3way] [--interactive] [--committer-date-is-author-date]
>  	 [--ignore-date] [--ignore-space-change | --ignore-whitespace]
>  	 [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
>  	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
> -	 [--scissors | --no-scissors]
> +	 [--[no-]scissors]
>  	 [(<mbox> | <Maildir>)...]
>  'git am' (--continue | --skip | --abort)
>  
> @@ -43,8 +43,7 @@ OPTIONS
>  --keep-non-patch::
>  	Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
>  
> ---keep-cr::
> ---no-keep-cr::
> +--[no-]keep-cr::
>  	With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1])
>  	with the same option, to prevent it from stripping CR at the end of
>  	lines. `am.keepcr` configuration variable can be used to specify the
OK.

We still have two separate entries for "--scissors/--no-scissors" and "--utf8/--no-utf8" pairs in the description text. Do they want to get united somehow?

Show 22 quoted lines
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index 5c16e31..a0727d7 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -14,7 +14,7 @@ SYNOPSIS
>  	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
>  	  [--separate-git-dir <git dir>]
>  	  [--depth <depth>] [--[no-]single-branch]
> -	  [--recursive|--recurse-submodules] [--] <repository>
> +	  [--recursive | --recurse-submodules] [--] <repository>
>  	  [<directory>]
>  
>  DESCRIPTION
> @@ -188,7 +188,7 @@ objects from the source repository into a pack in the cloned repository.
>  	with a long history, and would want to send in fixes
>  	as patches.
>  
> ---single-branch::
> +--[no-]single-branch::
>  	Clone only the history leading to the tip of a single branch,
>  	either specified by the `--branch` option or the primary
>  	branch remote's `HEAD` points at. When creating a shallow
OK.

We have "--no-hardlinks" without "--hardlinks". Does the parser accept "--no-no-hardlinks"? If so, we may want to error it out.

I would omit the options "--no-hardlinks" and "--shared", and added "--local={link,copy,shared}" instead, as these two options only make sense when cloning from a local repository, if I were doing the UI from scratch today.

Show 13 quoted lines
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index 8172938..1a7616c 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>  	   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]
>  	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
>  	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
> -	   [--date=<date>] [--cleanup=<mode>] [--status | --no-status]
> +	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
>  	   [-i | -o] [-S[<keyid>]] [--] [<file>...]
>  
>  DESCRIPTION
OK.

We have "--no-verify" without "--verify". Does the parser accept "--no-no-verify"? If so we may want to error it out.

Show 13 quoted lines
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index 9ae2508..d88a6fc 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -186,8 +186,7 @@ See also <<FILES>>.
>  	Opens an editor to modify the specified config file; either
>  	'--system', '--global', or repository (default).
>  
> ---includes::
> ---no-includes::
> +--[no-]includes::
>  	Respect `include.*` directives in config files when looking up
>  	values. Defaults to on.
OK.
The SYNOPSIS section begins like this:
    SYNOPSIS
    --------
    [verse]
    'git config' [<file-option>] [type] [-z|--null] name [value [value_regex]]
    ...

Other parts of this patch seems to want to have SPs around vertical bars that shows choices, so "[-z | --null]" would be necessary as a follow-up to bring more consistency.

The placeholders like "type", "name", "value", etc. are not inside <angle-brackets>, which may want to be fixed as well.

It is unclear what "<file-option>" is. A few paragraphs in the DESCRIPTION section talk about --system/global/etc.; there should be a sentence there to mention that exact term <file-option> to help the reader make a connection, just like the paragraph that talks about "type" uses that word to make it clear the word in SYNOPSIS is about things like --int/bool/path.

Show 11 quoted lines
> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index bfb106c..61a5701 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -17,7 +17,7 @@ SYNOPSIS
>  	     [--enable=<service>] [--disable=<service>]
>  	     [--allow-override=<service>] [--forbid-override=<service>]
>  	     [--access-hook=<path>]
> -	     [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>] [--user=<user> [--group=<group>]]
> +	     [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>] [--user=<user> [--group=<group>]]]
>  	     [<directory>...]

You can say "--inetd". Only when you do not say "--inetd", you can use "--listen", "--port", etc. Further, only when you say "--user", you can also say "--group". OK.

As this is governed by [verse], it would be more helpful to show that logic not just with the nesting of [], but make it stand out with a line break, perhaps like:

        [--inetd |
         [--listen=<host>] [--port=<n>]
         [--user=<user> [--group=<group>]]]
(I do not mind to see a single line for the second and third lines).
Show 10 quoted lines
> @@ -169,8 +169,7 @@ Git configuration files in that directory are readable by `<user>`.
>  	repository configuration.  By default, all the services
>  	are overridable.
>  
> ---informative-errors::
> ---no-informative-errors::
> +--[no-]informative-errors::
>  	When informative errors are turned on, git-daemon will report
>  	more verbose errors to the client, differentiating conditions
>  	like "no such repository" from "repository not exported". This
OK.

By the way, this is missing from SYNOPSIS, unlike all the other options to this command.

Show 14 quoted lines
> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
> index 8361e6e..11887e6 100644
> --- a/Documentation/git-difftool.txt
> +++ b/Documentation/git-difftool.txt
> @@ -69,8 +69,7 @@ with custom merge tool commands and has the same value as `$MERGED`.
>  --tool-help::
>  	Print a list of diff tools that may be used with `--tool`.
>  
> ---symlinks::
> ---no-symlinks::
> +--[no-]symlinks::
>  	'git difftool''s default behavior is create symlinks to the
>  	working tree when run in `--dir-diff` mode and the right-hand
>  	side of the comparison yields the same content as the file in
OK.

We have "--no-symlinks" without "--symlinks". Does the parser accept "--no-no-symlinks"? If so, we may want to error it out.

Show 18 quoted lines
> diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
> index 03fc8c3..efb0380 100644
> --- a/Documentation/git-fast-export.txt
> +++ b/Documentation/git-fast-export.txt
> @@ -106,11 +106,11 @@ marks the same across runs.
>  	different from the commit's first parent).
>  
>  [<git-rev-list-args>...]::
> -       A list of arguments, acceptable to 'git rev-parse' and
> -       'git rev-list', that specifies the specific objects and references
> -       to export.  For example, `master~10..master` causes the
> -       current master reference to be exported along with all objects
> -       added since its 10th ancestor commit.
> +	A list of arguments, acceptable to 'git rev-parse' and
> +	'git rev-list', that specifies the specific objects and references
> +	to export.  For example, `master~10..master` causes the
> +	current master reference to be exported along with all objects
> +	added since its 10th ancestor commit.
OK.

We have "--no-data" without "--data". Does the parser accept "--no-no-data"? If so, we may want to error it out.

Show 14 quoted lines
> diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
> index b81e90d..1e71754 100644
> --- a/Documentation/git-fetch-pack.txt
> +++ b/Documentation/git-fetch-pack.txt
> @@ -10,9 +10,9 @@ SYNOPSIS
>  --------
>  [verse]
>  'git fetch-pack' [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag]
> -				[--upload-pack=<git-upload-pack>]
> -				[--depth=<n>] [--no-progress]
> -				[-v] [<host>:]<directory> [<refs>...]
> +	[--upload-pack=<git-upload-pack>]
> +	[--depth=<n>] [--no-progress]
> +	[-v] [<host>:]<directory> [<refs>...]
OK.

We have "--no-progress" without "--progress". Does the parser accept "--no-no-progress"? If so, we may want to error it out.

Show 13 quoted lines
> diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
> index 6b563c5..d758f3a 100644
> --- a/Documentation/git-mergetool.txt
> +++ b/Documentation/git-mergetool.txt
> @@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve merge conflicts
>  SYNOPSIS
>  --------
>  [verse]
> -'git mergetool' [--tool=<tool>] [-y|--no-prompt|--prompt] [<file>...]
> +'git mergetool' [--tool=<tool>] [-y|--[no-]prompt] [<file>...]
>  
>  DESCRIPTION
>  -----------

Other parts of this patch seems to want to have SPs around vertical bars that shows choices, so "[-y | --[no-]prompt]" would be needed, especially given that this patch touches that same line.

Show 13 quoted lines
> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
> index 70152e8..f79c9d8 100644
> --- a/Documentation/git-revert.txt
> +++ b/Documentation/git-revert.txt
> @@ -8,7 +8,7 @@ git-revert - Revert some existing commits
>  SYNOPSIS
>  --------
>  [verse]
> -'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
> +'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] <commit>...
>  'git revert' --continue
>  'git revert' --quit
>  'git revert' --abort
OK.

The DESCRIPTION section only describes "--no-edit", which may need to be fixed.

-- squash --
    SQUASH???
    Update the overlong SYNOPSIS section for git-daemon. As the patch
    adds SPs around vertical bars for [choice1 | choice2], match the
    line it touches in git-mergetool.txt to also do that.
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 61a5701..223f731 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -16,8 +16,10 @@ SYNOPSIS
 	     [--reuseaddr] [--detach] [--pid-file=<file>]
 	     [--enable=<service>] [--disable=<service>]
 	     [--allow-override=<service>] [--forbid-override=<service>]
-	     [--access-hook=<path>]
-	     [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>] [--user=<user> [--group=<group>]]]
+	     [--access-hook=<path>] [--[no-]informative-errors]
+	     [--inetd |
+	      [--listen=<host_or_ipaddr>] [--port=<n>]
+	      [--user=<user> [--group=<group>]]]
 	     [<directory>...]
 
 DESCRIPTION
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index d758f3a..07137f2 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve merge conflicts
 SYNOPSIS
 --------
 [verse]
-'git mergetool' [--tool=<tool>] [-y|--[no-]prompt] [<file>...]
+'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [<file>...]
 
 DESCRIPTION
 -----------
Previous: Felipe ContrerasNext: Felipe Contreras
Message 3 of 40 in “trivial patches”
  1. 0/4 trivial patchesFelipe Contreras, May 9, 2013
  2. 1/4 documentation: trivial style cleanupsFelipe Contreras, May 9, 2013
  3. Junio C HamanoMay 9, 2013
  4. 2/4 transport-helper: trivial style cleanupFelipe Contreras, May 9, 2013
  5. Junio C HamanoMay 9, 2013
  6. 3/4 {fast-export,transport-helper}: style cleanupsFelipe Contreras, May 9, 2013
  7. John SzakmeisterMay 9, 2013
  8. Felipe ContrerasMay 9, 2013
  9. John SzakmeisterMay 9, 2013
  10. Felipe ContrerasMay 9, 2013
  11. Ramkumar RamachandraMay 9, 2013
  12. Felipe ContrerasMay 9, 2013
  13. Junio C HamanoMay 9, 2013
  14. Felipe ContrerasMay 9, 2013
  15. Junio C HamanoMay 9, 2013
  16. Felipe ContrerasMay 9, 2013
  17. Felipe ContrerasMay 16, 2013
  18. Junio C HamanoMay 16, 2013
  19. Felipe ContrerasMay 16, 2013
  20. Junio C HamanoMay 16, 2013
  21. Felipe ContrerasMay 16, 2013
  22. Junio C HamanoMay 16, 2013
  23. Ramkumar RamachandraMay 16, 2013
  24. Felipe ContrerasMay 16, 2013
  25. Felipe ContrerasMay 16, 2013
  26. Felipe ContrerasMay 17, 2013
  27. Ramkumar RamachandraMay 17, 2013
  28. Felipe ContrerasMay 17, 2013
  29. Junio C HamanoMay 17, 2013
  30. Felipe ContrerasMay 17, 2013
  31. Matthieu MoyMay 17, 2013
  32. Felipe ContrerasMay 17, 2013
  33. Matthieu MoyMay 17, 2013
  34. Felipe ContrerasMay 17, 2013
  35. 4/4 fast-export: trivial cleanupFelipe Contreras, May 9, 2013
  36. John SzakmeisterMay 9, 2013
  37. Felipe ContrerasMay 9, 2013
  38. John SzakmeisterMay 9, 2013
  39. Junio C HamanoMay 9, 2013
  40. Felipe ContrerasMay 16, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.