{"thread":{"id":"35535","subject":"[PATCH v2 1/2] diff: add test for --no-index executed outside repo","startedAt":"2013-12-16T20:19:23Z","lastAt":"2013-12-16T20:19:24Z","messageCount":2,"participants":["Thomas Gummerer"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"232081","messageId":"1387225164-31747-1-git-send-email-t.gummerer@gmail.com","threadId":"35535","inReplyTo":"xmqqob4goa4t.fsf@gitster.dls.corp.google.co","subject":"[PATCH v2 1/2] diff: add test for --no-index executed outside repo","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2013-12-16T20:19:23Z","receivedAt":"2013-12-16T20:19:23Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"470faf9 diff: move no-index detection to builtin/diff.c breaks the error\nmessage for \"git diff --no-index\", when the command is executed outside\nof a git repository and the wrong number of arguments are given. 6df5762\ndiff: don't read index when --no-index is given fixes the problem.\n\nAdd a test to guard against similar breakages in the future.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n\n> Perhaps use CEILING, like this (untested)?\n\nThanks, this makes sense, here is the new version.\n\n t/t4053-diff-no-index.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 077c775..2ab3c48 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -44,4 +44,15 @@ test_expect_success 'git diff outside repo with broken index' '\n \t)\n '\n \n+test_expect_success 'git diff --no-index executed outside repo gives correct error message' '\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=$TRASH_DIRECTORY/non &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git diff --no-index a 2>actual.err &&\n+\t\techo \"usage: git diff --no-index <path> <path>\" >expect.err &&\n+\t\ttest_cmp expect.err actual.err\n+\t)\n+'\n+\n test_done\n-- \n1.8.5.4.g8639e57\n"},{"id":"232082","messageId":"1387225164-31747-2-git-send-email-t.gummerer@gmail.com","threadId":"35535","inReplyTo":"1387225164-31747-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v2 2/2] diff: avoid some nesting","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2013-12-16T20:19:24Z","receivedAt":"2013-12-16T20:19:24Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Avoid some nesting in builtin/diff.c, to make the code easier to read.\nThere are no functional changes.\n\nHelped-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n builtin/diff.c | 35 +++++++++++++++++------------------\n 1 file changed, 17 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex ea1dd65..24d6271 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -319,27 +319,26 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n \tinit_revisions(&rev, prefix);\n \n-\tif (no_index) {\n-\t\tif (argc != i + 2) {\n-\t\t\tif (no_index == DIFF_NO_INDEX_IMPLICIT) {\n-\t\t\t\t/*\n-\t\t\t\t * There was no --no-index and there were not two\n-\t\t\t\t * paths. It is possible that the user intended\n-\t\t\t\t * to do an inside-repository operation.\n-\t\t\t\t */\n-\t\t\t\tfprintf(stderr, \"Not a git repository\\n\");\n-\t\t\t\tfprintf(stderr,\n-\t\t\t\t\t\"To compare two paths outside a working tree:\\n\");\n-\t\t\t}\n-\t\t\t/* Give the usage message for non-repository usage and exit. */\n-\t\t\tusagef(\"git diff %s <path> <path>\",\n-\t\t\t       no_index == DIFF_NO_INDEX_EXPLICIT ?\n-\t\t\t\t\t\"--no-index\" : \"[--no-index]\");\n-\n+\tif (no_index && argc != i + 2) {\n+\t\tif (no_index == DIFF_NO_INDEX_IMPLICIT) {\n+\t\t\t/*\n+\t\t\t * There was no --no-index and there were not two\n+\t\t\t * paths. It is possible that the user intended\n+\t\t\t * to do an inside-repository operation.\n+\t\t\t */\n+\t\t\tfprintf(stderr, \"Not a git repository\\n\");\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"To compare two paths outside a working tree:\\n\");\n \t\t}\n+\t\t/* Give the usage message for non-repository usage and exit. */\n+\t\tusagef(\"git diff %s <path> <path>\",\n+\t\t       no_index == DIFF_NO_INDEX_EXPLICIT ?\n+\t\t       \"--no-index\" : \"[--no-index]\");\n+\n+\t}\n+\tif (no_index)\n \t\t/* If this is a no-index diff, just run it and exit there. */\n \t\tdiff_no_index(&rev, argc, argv, prefix);\n-\t}\n \n \t/* Otherwise, we are doing the usual \"git\" diff */\n \trev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;\n-- \n1.8.5.4.g8639e57\n"}]}