{"thread":{"id":"53679","subject":"[PATCH] builtin/diff: fix incorrect comment","startedAt":"2020-06-15T11:15:22Z","lastAt":"2020-06-18T10:43:50Z","messageCount":3,"participants":["Denton Liu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"399758","messageId":"a9aea5dbb821544eea636bc5de9fe8185bb603fb.1592219700.git.liu.denton@gmail.com","threadId":"53679","inReplyTo":null,"subject":"[PATCH] builtin/diff: fix incorrect comment","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-06-15T11:15:07Z","receivedAt":"2020-06-15T11:15:22Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"A comment in cmd_diff() states that if one tree-ish and no blobs are\nprovided, it would provide a diff between the tree and the cache. This\nis incorrect because a diff happens between the tree-ish and the working\ntree. Remove the `--cached` in the comment so that the correct behavior\nis shown.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n builtin/diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 8537b17bd5..1ebab58c55 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -276,7 +276,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t *\tcompare a blob with a working tree file.\n \t *\n \t * N=1, M=0:\n-\t *      tree vs cache (diff-index --cached)\n+\t *      tree vs files (diff-index)\n \t *\n \t * N=2, M=0:\n \t *      tree vs tree (diff-tree)\n-- \n2.27.0.132.g321788e831\n\n"},{"id":"399983","messageId":"xmqqzh91va5n.fsf@gitster.c.googlers.com","threadId":"53679","inReplyTo":"a9aea5dbb821544eea636bc5de9fe8185bb603fb.1592219700.git.liu.denton@gmail.com","subject":"Re: [PATCH] builtin/diff: fix incorrect comment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-17T18:07:16Z","receivedAt":"2020-06-17T18:07:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> A comment in cmd_diff() states that if one tree-ish and no blobs are\n> provided, it would provide a diff between the tree and the cache. This\n> is incorrect because a diff happens between the tree-ish and the working\n> tree. Remove the `--cached` in the comment so that the correct behavior\n> is shown.\n\nPerhaps \"diff-index [--cached]\" is more appropriate, then?  After\nall, \"git diff --cached HEAD\" would be N=1 M=0 case, no?\n\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n>  builtin/diff.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/diff.c b/builtin/diff.c\n> index 8537b17bd5..1ebab58c55 100644\n> --- a/builtin/diff.c\n> +++ b/builtin/diff.c\n> @@ -276,7 +276,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n>  \t *\tcompare a blob with a working tree file.\n>  \t *\n>  \t * N=1, M=0:\n> -\t *      tree vs cache (diff-index --cached)\n> +\t *      tree vs files (diff-index)\n>  \t *\n>  \t * N=2, M=0:\n>  \t *      tree vs tree (diff-tree)\n"},{"id":"400053","messageId":"54c5de021a50ca7f39588efde1b89119aaf4cffa.1592476980.git.liu.denton@gmail.com","threadId":"53679","inReplyTo":"a9aea5dbb821544eea636bc5de9fe8185bb603fb.1592219700.git.liu.denton@gmail.com","subject":"[PATCH v2] builtin/diff: update usage comment","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-06-18T10:43:34Z","receivedAt":"2020-06-18T10:43:50Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"A comment in cmd_diff() states that if one tree-ish and no blobs are\nprovided, (the \"N=1, M=0\" case), it will provide a diff between the tree\nand the cache. This is incorrect because a diff happens between the\ntree-ish and the working tree. Remove the `--cached` in the comment so\nthat the correct behavior is shown. Add a new section describing the\n\"N=1, M=0, --cached\" behavior.\n\nNext, describe the \"N=0, M=0, --cached\" case, similar to the above since\nit is undocumented.\n\nFinally, fix some spacing issues. Add spaces between each section for\nconsistency and readability. Also, change tabs within the comment into\nspaces.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n\nNotes:\n    This change can be broken down into three separate commits but I figured\n    it is a little bit overkill considering how little the scope of each\n    individual change would be. If it's more readable as separate commits, I\n    can resubmit it, though.\n\nRange-diff against v1:\n1:  a9aea5dbb8 ! 1:  54c5de021a builtin/diff: fix incorrect comment\n    @@ Metadata\n     Author: Denton Liu <liu.denton@gmail.com>\n     \n      ## Commit message ##\n    -    builtin/diff: fix incorrect comment\n    +    builtin/diff: update usage comment\n     \n         A comment in cmd_diff() states that if one tree-ish and no blobs are\n    -    provided, it would provide a diff between the tree and the cache. This\n    -    is incorrect because a diff happens between the tree-ish and the working\n    -    tree. Remove the `--cached` in the comment so that the correct behavior\n    -    is shown.\n    +    provided, (the \"N=1, M=0\" case), it will provide a diff between the tree\n    +    and the cache. This is incorrect because a diff happens between the\n    +    tree-ish and the working tree. Remove the `--cached` in the comment so\n    +    that the correct behavior is shown. Add a new section describing the\n    +    \"N=1, M=0, --cached\" behavior.\n    +\n    +    Next, describe the \"N=0, M=0, --cached\" case, similar to the above since\n    +    it is undocumented.\n    +\n    +    Finally, fix some spacing issues. Add spaces between each section for\n    +    consistency and readability. Also, change tabs within the comment into\n    +    spaces.\n    +\n    +\n    + ## Notes ##\n    +    This change can be broken down into three separate commits but I figured\n    +    it is a little bit overkill considering how little the scope of each\n    +    individual change would be. If it's more readable as separate commits, I\n    +    can resubmit it, though.\n     \n      ## builtin/diff.c ##\n     @@ builtin/diff.c: int cmd_diff(int argc, const char **argv, const char *prefix)\n    - \t *\tcompare a blob with a working tree file.\n    + \n    + \t/*\n    + \t * We could get N tree-ish in the rev.pending_objects list.\n    +-\t * Also there could be M blobs there, and P pathspecs.\n    ++\t * Also there could be M blobs there, and P pathspecs. --cached may\n    ++\t * also be present.\n      \t *\n    - \t * N=1, M=0:\n    --\t *      tree vs cache (diff-index --cached)\n    + \t * N=0, M=0:\n    +-\t *\tcache vs files (diff-files)\n    ++\t *      cache vs files (diff-files)\n    ++\t *\n    ++\t * N=0, M=0, --cached:\n    ++\t *      HEAD vs cache (diff-index --cached)\n    ++\t *\n    + \t * N=0, M=2:\n    + \t *      compare two random blobs.  P must be zero.\n    ++\t *\n    + \t * N=0, M=1, P=1:\n    +-\t *\tcompare a blob with a working tree file.\n    ++\t *      compare a blob with a working tree file.\n    ++\t *\n    ++\t * N=1, M=0:\n     +\t *      tree vs files (diff-index)\n      \t *\n    + \t * N=1, M=0:\n    + \t *      tree vs cache (diff-index --cached)\n    + \t *\n    ++\t * N=1, M=0, --cached:\n    ++\t *      tree vs files (diff-index)\n    ++\t *\n      \t * N=2, M=0:\n      \t *      tree vs tree (diff-tree)\n    + \t *\n\n builtin/diff.c | 18 +++++++++++++++---\n 1 file changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 8537b17bd5..0f1a882bd7 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -266,18 +266,30 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n \t/*\n \t * We could get N tree-ish in the rev.pending_objects list.\n-\t * Also there could be M blobs there, and P pathspecs.\n+\t * Also there could be M blobs there, and P pathspecs. --cached may\n+\t * also be present.\n \t *\n \t * N=0, M=0:\n-\t *\tcache vs files (diff-files)\n+\t *      cache vs files (diff-files)\n+\t *\n+\t * N=0, M=0, --cached:\n+\t *      HEAD vs cache (diff-index --cached)\n+\t *\n \t * N=0, M=2:\n \t *      compare two random blobs.  P must be zero.\n+\t *\n \t * N=0, M=1, P=1:\n-\t *\tcompare a blob with a working tree file.\n+\t *      compare a blob with a working tree file.\n+\t *\n+\t * N=1, M=0:\n+\t *      tree vs files (diff-index)\n \t *\n \t * N=1, M=0:\n \t *      tree vs cache (diff-index --cached)\n \t *\n+\t * N=1, M=0, --cached:\n+\t *      tree vs files (diff-index)\n+\t *\n \t * N=2, M=0:\n \t *      tree vs tree (diff-tree)\n \t *\n-- \n2.27.0.132.g321788e831\n\n"}]}