{"thread":{"id":"52988","subject":"[RFC PATCH] Make rev-parse -q and --is-* quiet","startedAt":"2020-03-13T12:13:58Z","lastAt":"2020-03-13T17:52:39Z","messageCount":2,"participants":["Erlend Aasland","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"393199","messageId":"4B2A0C64-FD4A-457C-A7CF-5B680AF38BC9@innova.no","threadId":"52988","inReplyTo":null,"subject":"[RFC PATCH] Make rev-parse -q and --is-* quiet","fromName":"Erlend Aasland","fromEmail":"erlend-a@innova.no","sentAt":"2020-03-13T12:13:52Z","receivedAt":"2020-03-13T12:13:58Z","isPatch":true,"sender":{"key":"erlend-a@innova.no","avatar":null},"body":"If rev-parse is called with both -q and an --is-* option, the result is\nprovided as the return code of the command, iso. printed to stdout.\n\nThis simplifies using these queries in shell scripts:\ngit rev-parse --is-bare-repository && do_stuff\ngit rev-parse --is-shallow-repository && do_stuff\n\nSigned-off-by: Erlend E. Aasland <erlend.aasland@innova.no>\n---\nbuiltin/rev-parse.c | 25 ++++++++++++++++---------\n1 file changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 7a00da8203..5a8b404ec7 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -874,24 +874,31 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n\t\t\t\tcontinue;\n\t\t\t}\n\t\t\tif (!strcmp(arg, \"--is-inside-git-dir\")) {\n-\t\t\t\tprintf(\"%s\\n\", is_inside_git_dir() ? \"true\"\n-\t\t\t\t\t\t: \"false\");\n+\t\t\t\tint is_set = is_inside_git_dir();\n+\t\t\t\tif (quiet)\n+\t\t\t\t\treturn is_set ? 0 : 1;\n+\t\t\t\tprintf(\"%s\\n\", is_set ? \"true\" : \"false\");\n\t\t\t\tcontinue;\n\t\t\t}\n\t\t\tif (!strcmp(arg, \"--is-inside-work-tree\")) {\n-\t\t\t\tprintf(\"%s\\n\", is_inside_work_tree() ? \"true\"\n-\t\t\t\t\t\t: \"false\");\n+\t\t\t\tint is_set = is_inside_work_tree();\n+\t\t\t\tif (quiet)\n+\t\t\t\t\treturn is_set ? 0 : 1;\n+\t\t\t\tprintf(\"%s\\n\", is_set ? \"true\" : \"false\");\n\t\t\t\tcontinue;\n\t\t\t}\n\t\t\tif (!strcmp(arg, \"--is-bare-repository\")) {\n-\t\t\t\tprintf(\"%s\\n\", is_bare_repository() ? \"true\"\n-\t\t\t\t\t\t: \"false\");\n+\t\t\t\tint is_set = is_bare_repository();\n+\t\t\t\tif (quiet)\n+\t\t\t\t\treturn is_set ? 0 : 1;\n+\t\t\t\tprintf(\"%s\\n\", is_set ? \"true\" : \"false\");\n\t\t\t\tcontinue;\n\t\t\t}\n\t\t\tif (!strcmp(arg, \"--is-shallow-repository\")) {\n-\t\t\t\tprintf(\"%s\\n\",\n-\t\t\t\t\t\tis_repository_shallow(the_repository) ? \"true\"\n-\t\t\t\t\t\t: \"false\");\n+\t\t\t\tint is_set = is_repository_shallow(the_repository);\n+\t\t\t\tif (quiet)\n+\t\t\t\t\treturn is_set ? 0 : 1;\n+\t\t\t\tprintf(\"%s\\n\", is_set ? \"true\" : \"false\");\n\t\t\t\tcontinue;\n\t\t\t}\n\t\t\tif (!strcmp(arg, \"--shared-index-path\")) {\n-- \n2.25.1\n\n"},{"id":"393220","messageId":"20200313175236.GB549554@coredump.intra.peff.net","threadId":"52988","inReplyTo":"4B2A0C64-FD4A-457C-A7CF-5B680AF38BC9@innova.no","subject":"Re: [RFC PATCH] Make rev-parse -q and --is-* quiet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-13T17:52:36Z","receivedAt":"2020-03-13T17:52:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 13, 2020 at 12:13:52PM +0000, Erlend Aasland wrote:\n\n> If rev-parse is called with both -q and an --is-* option, the result is\n> provided as the return code of the command, iso. printed to stdout.\n> \n> This simplifies using these queries in shell scripts:\n> git rev-parse --is-bare-repository && do_stuff\n> git rev-parse --is-shallow-repository && do_stuff\n\nI left some comments in the other part of the thread, but another\npotential downside here is that the exit code also tells us whether the\ncommand was unable to run for unrelated reasons (i.e., the answer is\nreally a tristate: yes, no, or \"we don't know the answer\").\n\nOne obvious reason to exit non-zero would be that we're not in a\nrepository at all. I _think_ that works OK with all of these\nconditionals, because if we're not in a repo, then we are obviously not\ninside a git-dir, and so on.\n\nAnother reason is that the caller misspelled the option name. :) That\nmight be an acceptable risk, though. It's not like it isn't present in\nother commands, probably people are doing:\n\n  test \"$(git rev-parse --is-whatever\") = \"true\" && do_stuff\n\nanyway, which has the same problem. Though perhaps the simplicity there\nis an argument that we don't need the new exit-code mechanism.\n\n-Peff\n"}]}