threads / patch / 49483

patchdoc: fix a typo and clarify a sentence

Subject: [PATCH] doc: fix a typo and clarify a sentence

## tl;dr

7 messages between Oct 3, 2018 and Oct 10, 2018. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Mihir Mehta· Oct 3, 2018, 21:08 UTC · lore

I noticed that git-merge-base was unlikely to actually be a git command, and tried it in my shell. Seeing that it doesn't work, I cleaned up two places in the docs where it appears.

Signed-off-by: Mihir Mehta <mihir@cs.utexas.edu>
---
 Documentation/git-diff.txt                  | 5 +++--
 Documentation/howto/update-hook-example.txt | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

Show changes to 2 files +4 −3

Documentation/git-diff.txt, Documentation/howto/update-hook-example.txt

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index b180f1fa5..6173f569e 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -72,8 +72,9 @@ two blob objects, or changes between two files on disk.
 	This form is to view the changes on the branch containing
 	and up to the second <commit>, starting at a common ancestor
 	of both <commit>.  "git diff A\...B" is equivalent to
-	"git diff $(git-merge-base A B) B".  You can omit any one
-	of <commit>, which has the same effect as using HEAD instead.
+	"git diff $(git merge-base A B) B".  You can omit any one
+	of the two instances of <commit>, which has the same effect as
+	using HEAD in its place.
 
 Just in case if you are doing something exotic, it should be
 noted that all of the <commit> in the above description, except
diff --git a/Documentation/howto/update-hook-example.txt b/Documentation/howto/update-hook-example.txt
index a5193b1e5..89821ec74 100644
--- a/Documentation/howto/update-hook-example.txt
+++ b/Documentation/howto/update-hook-example.txt
@@ -80,7 +80,7 @@ case "$1" in
       info "The branch '$1' is new..."
     else
       # updating -- make sure it is a fast-forward
-      mb=$(git-merge-base "$2" "$3")
+      mb=$(git merge-base "$2" "$3")
       case "$mb,$2" in
         "$2,$mb") info "Update is fast-forward" ;;
 	*)	  noff=y; info "This is not a fast-forward update.";;

-- 
2.19.0

Junio C Hamano· Oct 5, 2018, 06:20 UTC · re: Mihir Mehta · lore

Re: [PATCH] doc: fix a typo and clarify a sentence

Mihir Mehta <mihir@cs.utexas.edu> writes:

Show 11 quoted lines

> diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
> index b180f1fa5..6173f569e 100644
> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -72,8 +72,9 @@ two blob objects, or changes between two files on disk.
>  	This form is to view the changes on the branch containing
>  	and up to the second <commit>, starting at a common ancestor
>  	of both <commit>.  "git diff A\...B" is equivalent to
> -	"git diff $(git-merge-base A B) B".  You can omit any one
> -	of <commit>, which has the same effect as using HEAD instead.
> +	"git diff $(git merge-base A B) B".  You can omit any one

"git merge-base" is a more modern way to spell "git-merge-base" and we have been trying to update the mention of the latter in the docs to the former. Thanks for doing this.

> +	of the two instances of <commit>, which has the same effect as

The paragraph is about <commit>...<commit> three-dot notation. I suspect that you wanted to say <commit>... and ...<commit> are allowed, implying that a bare ... is not allowed and does not mean the same thing as what HEAD...HEAD means. But that is not the case. Asking "git diff HEAD...HEAD" by omitting both may not give very interesting output (it always becomes a no-op), but nevertheless it is a valid thing to ask (iow "git diff $commit1...$commit2" is what you can safely write without having to worry about one or both going empty string). So I'd rather not to see this change in this form. It is an incomplete attempt to discourage use of <empty>...<empty> but without giving enough justification.

	Side note.  I am not recommending to do so, but
	"discouragement with enough justification" would look like
	this.

	You can omit <commit> on any side of the three dots, which
	has the same effect as using HEAD instead.  Omitting both
	and leaving only three dots is not an error but that merely
	specifies a set of commits that are and are not reachable
	from HEAD at the same time, which by definition is an empty
	set, hence it is not very useful.

> +	using HEAD in its place.

Show 7 quoted lines

> +++ b/Documentation/howto/update-hook-example.txt
> @@ -80,7 +80,7 @@ case "$1" in
>        info "The branch '$1' is new..."
>      else
>        # updating -- make sure it is a fast-forward
> -      mb=$(git-merge-base "$2" "$3")
> +      mb=$(git merge-base "$2" "$3")

I strongly suspect that inside update-hook, the original still should work (iow, $GIT_EXEC_PATH should already have been prepended to $PATH before a hoook is called). But the updated form should also work, and it is the form we humans need to type, so let's take this change.

Thanks.

>        case "$mb,$2" in
>          "$2,$mb") info "Update is fast-forward" ;;
>  	*)	  noff=y; info "This is not a fast-forward update.";;

Mihir Mehta· Oct 10, 2018, 15:20 UTC · re: Junio C Hamano · lore

Thanks, Junio. Instead of removing that part of the patch, I opted to expand it to make it a little clearer (in my opinion) than it was before. Let me know if this works.

Mihir.

Mihir Mehta· Oct 10, 2018, 15:20 UTC · re: Mihir Mehta · lore

I noticed that git-merge-base was unlikely to actually be a git command, and tried it in my shell. Seeing that it doesn't work, I cleaned up two places in the docs where it appears.

Signed-off-by: Mihir Mehta <mihir@cs.utexas.edu>
---
 Documentation/git-diff.txt                  | 7 ++++---
 Documentation/howto/update-hook-example.txt | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

Show changes to 2 files +5 −4

Documentation/git-diff.txt, Documentation/howto/update-hook-example.txt

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index b180f1fa5..a122f9ded 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -72,10 +72,11 @@ two blob objects, or changes between two files on disk.
 	This form is to view the changes on the branch containing
 	and up to the second <commit>, starting at a common ancestor
 	of both <commit>.  "git diff A\...B" is equivalent to
-	"git diff $(git-merge-base A B) B".  You can omit any one
-	of <commit>, which has the same effect as using HEAD instead.
+	"git diff $(git merge-base A B) B".  You can omit any one
+	of the two instances of <commit>, which has the same effect as
+	using HEAD in its place; omitting both results in an empty diff.
 
-Just in case if you are doing something exotic, it should be
+Just in case you are doing something exotic, it should be
 noted that all of the <commit> in the above description, except
 in the last two forms that use ".." notations, can be any
 <tree>.
diff --git a/Documentation/howto/update-hook-example.txt b/Documentation/howto/update-hook-example.txt
index a5193b1e5..89821ec74 100644
--- a/Documentation/howto/update-hook-example.txt
+++ b/Documentation/howto/update-hook-example.txt
@@ -80,7 +80,7 @@ case "$1" in
       info "The branch '$1' is new..."
     else
       # updating -- make sure it is a fast-forward
-      mb=$(git-merge-base "$2" "$3")
+      mb=$(git merge-base "$2" "$3")
       case "$mb,$2" in
         "$2,$mb") info "Update is fast-forward" ;;
 	*)	  noff=y; info "This is not a fast-forward update.";;

-- 
2.19.0

Junio C Hamano· Oct 10, 2018, 22:19 UTC · re: Mihir Mehta · lore

Re: none

Mihir Mehta <mihir@cs.utexas.edu> writes:

> Thanks, Junio. Instead of removing that part of the patch, I opted to
> expand it to make it a little clearer (in my opinion) than it was
> before. Let me know if this works.

I am mildly negative on that change. "Omitting both would give an empty diff" would be understandable to anybody who understands that an omitted end of dot-dot is substituted with HEAD *and* thinks what range HEAD..HEAD means, so it is just an additional noise to them, and to those who do not want to waste time on thinking, it is a statement that reads as if "it will be an error" without saying why it is an error. So overall, it seems, at least to me, that the additional text adds negative value.

So, I dunno.

Mihir Mehta· Oct 10, 2018, 22:26 UTC · re: Junio C Hamano · lore

I noticed that git-merge-base was unlikely to actually be a git command, and tried it in my shell. Seeing that it doesn't work, I cleaned up two places in the docs where it appears.

Signed-off-by: Mihir Mehta <mihir@cs.utexas.edu>
---
 Documentation/git-diff.txt                  | 4 ++--
 Documentation/howto/update-hook-example.txt | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Show changes to 2 files +3 −3

Documentation/git-diff.txt, Documentation/howto/update-hook-example.txt

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index b180f1fa5..030f162f3 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -72,10 +72,10 @@ two blob objects, or changes between two files on disk.
 	This form is to view the changes on the branch containing
 	and up to the second <commit>, starting at a common ancestor
 	of both <commit>.  "git diff A\...B" is equivalent to
-	"git diff $(git-merge-base A B) B".  You can omit any one
+	"git diff $(git merge-base A B) B".  You can omit any one
 	of <commit>, which has the same effect as using HEAD instead.
 
-Just in case if you are doing something exotic, it should be
+Just in case you are doing something exotic, it should be
 noted that all of the <commit> in the above description, except
 in the last two forms that use ".." notations, can be any
 <tree>.
diff --git a/Documentation/howto/update-hook-example.txt b/Documentation/howto/update-hook-example.txt
index a5193b1e5..89821ec74 100644
--- a/Documentation/howto/update-hook-example.txt
+++ b/Documentation/howto/update-hook-example.txt
@@ -80,7 +80,7 @@ case "$1" in
       info "The branch '$1' is new..."
     else
       # updating -- make sure it is a fast-forward
-      mb=$(git-merge-base "$2" "$3")
+      mb=$(git merge-base "$2" "$3")
       case "$mb,$2" in
         "$2,$mb") info "Update is fast-forward" ;;
 	*)	  noff=y; info "This is not a fast-forward update.";;

-- 
2.19.0

Junio C Hamano· Oct 10, 2018, 23:27 UTC · re: Mihir Mehta · lore

Re: [PATCH] doc: fix a typo and clarify a sentence

Mihir Mehta <mihir@cs.utexas.edu> writes:

> -Just in case if you are doing something exotic, it should be
> +Just in case you are doing something exotic, it should be

Thanks. Somehow I didn't notice this change earlier, but it looks good, too.

Will queue.