{"thread":{"id":"37457","subject":"[PATCH 1/2] bisect: remove unnecessary redirection","startedAt":"2014-08-30T19:30:28Z","lastAt":"2014-08-31T07:35:51Z","messageCount":5,"participants":["David Aguilar","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"248656","messageId":"1409427029-65886-1-git-send-email-davvid@gmail.com","threadId":"37457","inReplyTo":null,"subject":"[PATCH 1/2] bisect: remove unnecessary redirection","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-08-30T19:30:28Z","receivedAt":"2014-08-30T19:30:28Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"`git rev-parse` is being called with --quiet so there's no need to\nredirect to /dev/null.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-bisect.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 1e0d602..c1c2321 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -365,7 +365,7 @@ bisect_reset() {\n \t}\n \tcase \"$#\" in\n \t0) branch=$(cat \"$GIT_DIR/BISECT_START\") ;;\n-\t1) git rev-parse --quiet --verify \"$1^{commit}\" >/dev/null || {\n+\t1) git rev-parse --quiet --verify \"$1^{commit}\" || {\n \t\t\tinvalid=\"$1\"\n \t\t\tdie \"$(eval_gettext \"'\\$invalid' is not a valid commit\")\"\n \t\t}\n-- \n2.1.0.29.g9b751c4\n"},{"id":"248657","messageId":"1409427029-65886-2-git-send-email-davvid@gmail.com","threadId":"37457","inReplyTo":"1409427029-65886-1-git-send-email-davvid@gmail.com","subject":"[PATCH 2/2] stash: prefer --quiet over shell redirection","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-08-30T19:30:29Z","receivedAt":"2014-08-30T19:30:29Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Use `git rev-parse --quiet` instead of redirecting to /dev/null.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-stash.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex bcc757b..5a5185b 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -50,7 +50,7 @@ clear_stash () {\n \tthen\n \t\tdie \"$(gettext \"git stash clear with parameters is unimplemented\")\"\n \tfi\n-\tif current=$(git rev-parse --verify $ref_stash 2>/dev/null)\n+\tif current=$(git rev-parse --quiet --verify $ref_stash)\n \tthen\n \t\tgit update-ref -d $ref_stash $current\n \tfi\n@@ -67,7 +67,7 @@ create_stash () {\n \tfi\n \n \t# state of the base commit\n-\tif b_commit=$(git rev-parse --verify HEAD)\n+\tif b_commit=$(git rev-parse --quiet --verify HEAD)\n \tthen\n \t\thead=$(git rev-list --oneline -n 1 HEAD --)\n \telse\n@@ -292,7 +292,7 @@ save_stash () {\n }\n \n have_stash () {\n-\tgit rev-parse --verify $ref_stash >/dev/null 2>&1\n+\tgit rev-parse --quiet --verify $ref_stash\n }\n \n list_stash () {\n@@ -392,12 +392,12 @@ parse_flags_and_rev()\n \t\t;;\n \tesac\n \n-\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\" 2>/dev/null) || {\n+\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\") || {\n \t\treference=\"$1\"\n \t\tdie \"$(eval_gettext \"\\$reference is not valid reference\")\"\n \t}\n \n-\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\" 2>/dev/null) &&\n+\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\") &&\n \tset -- $(git rev-parse \"$REV\" \"$REV^1\" \"$REV:\" \"$REV^1:\" \"$REV^2:\" 2>/dev/null) &&\n \ts=$1 &&\n \tw_commit=$1 &&\n@@ -409,7 +409,7 @@ parse_flags_and_rev()\n \ttest \"$ref_stash\" = \"$(git rev-parse --symbolic-full-name \"${REV%@*}\")\" &&\n \tIS_STASH_REF=t\n \n-\tu_commit=$(git rev-parse --quiet --verify \"$REV^3\" 2>/dev/null) &&\n+\tu_commit=$(git rev-parse --quiet --verify \"$REV^3\") &&\n \tu_tree=$(git rev-parse \"$REV^3:\" 2>/dev/null)\n }\n \n@@ -531,7 +531,7 @@ drop_stash () {\n \t\tdie \"$(eval_gettext \"\\${REV}: Could not drop stash entry\")\"\n \n \t# clear_stash if we just dropped the last stash entry\n-\tgit rev-parse --verify \"$ref_stash@{0}\" >/dev/null 2>&1 || clear_stash\n+\tgit rev-parse --quiet --verify \"$ref_stash@{0}\" || clear_stash\n }\n \n apply_to_branch () {\n-- \n2.1.0.29.g9b751c4\n"},{"id":"248662","messageId":"54023AA1.40709@kdbg.org","threadId":"37457","inReplyTo":"1409427029-65886-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 1/2] bisect: remove unnecessary redirection","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-08-30T20:57:05Z","receivedAt":"2014-08-30T20:57:05Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 30.08.2014 21:30, schrieb David Aguilar:\n> `git rev-parse` is being called with --quiet so there's no need to\n> redirect to /dev/null.\n> \n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n>  git-bisect.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index 1e0d602..c1c2321 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -365,7 +365,7 @@ bisect_reset() {\n>  \t}\n>  \tcase \"$#\" in\n>  \t0) branch=$(cat \"$GIT_DIR/BISECT_START\") ;;\n> -\t1) git rev-parse --quiet --verify \"$1^{commit}\" >/dev/null || {\n> +\t1) git rev-parse --quiet --verify \"$1^{commit}\" || {\n\nThis is wrong. The redirection quells stdout, but --quiet is about stderr.\n\n>  \t\t\tinvalid=\"$1\"\n>  \t\t\tdie \"$(eval_gettext \"'\\$invalid' is not a valid commit\")\"\n>  \t\t}\n> \n\n-- Hannes\n"},{"id":"248663","messageId":"54023D01.6090504@kdbg.org","threadId":"37457","inReplyTo":"1409427029-65886-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 2/2] stash: prefer --quiet over shell redirection","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-08-30T21:07:13Z","receivedAt":"2014-08-30T21:07:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 30.08.2014 21:30, schrieb David Aguilar:\n> Use `git rev-parse --quiet` instead of redirecting to /dev/null.\n> \n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n>  git-stash.sh | 14 +++++++-------\n>  1 file changed, 7 insertions(+), 7 deletions(-)\n> \n> diff --git a/git-stash.sh b/git-stash.sh\n> index bcc757b..5a5185b 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -50,7 +50,7 @@ clear_stash () {\n>  \tthen\n>  \t\tdie \"$(gettext \"git stash clear with parameters is unimplemented\")\"\n>  \tfi\n> -\tif current=$(git rev-parse --verify $ref_stash 2>/dev/null)\n> +\tif current=$(git rev-parse --quiet --verify $ref_stash)\n>  \tthen\n>  \t\tgit update-ref -d $ref_stash $current\n>  \tfi\n> @@ -67,7 +67,7 @@ create_stash () {\n>  \tfi\n>  \n>  \t# state of the base commit\n> -\tif b_commit=$(git rev-parse --verify HEAD)\n> +\tif b_commit=$(git rev-parse --quiet --verify HEAD)\n>  \tthen\n>  \t\thead=$(git rev-list --oneline -n 1 HEAD --)\n>  \telse\n\nThe else branch calls die; wouldn't it be useful to keep the error\nmessage of rev-parse?\n\n> @@ -292,7 +292,7 @@ save_stash () {\n>  }\n>  \n>  have_stash () {\n> -\tgit rev-parse --verify $ref_stash >/dev/null 2>&1\n> +\tgit rev-parse --quiet --verify $ref_stash\n\nThis change is not correct: --quiet removes only output on stderr, but\nnot that on stdout.\n\n>  }\n>  \n>  list_stash () {\n> @@ -392,12 +392,12 @@ parse_flags_and_rev()\n>  \t\t;;\n>  \tesac\n>  \n> -\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\" 2>/dev/null) || {\n> +\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\") || {\n>  \t\treference=\"$1\"\n>  \t\tdie \"$(eval_gettext \"\\$reference is not valid reference\")\"\n>  \t}\n>  \n> -\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\" 2>/dev/null) &&\n> +\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\") &&\n>  \tset -- $(git rev-parse \"$REV\" \"$REV^1\" \"$REV:\" \"$REV^1:\" \"$REV^2:\" 2>/dev/null) &&\n\nI see another rev-parse that you did not modify. An omission?\n\n>  \ts=$1 &&\n>  \tw_commit=$1 &&\n> @@ -409,7 +409,7 @@ parse_flags_and_rev()\n>  \ttest \"$ref_stash\" = \"$(git rev-parse --symbolic-full-name \"${REV%@*}\")\" &&\n>  \tIS_STASH_REF=t\n>  \n> -\tu_commit=$(git rev-parse --quiet --verify \"$REV^3\" 2>/dev/null) &&\n> +\tu_commit=$(git rev-parse --quiet --verify \"$REV^3\") &&\n>  \tu_tree=$(git rev-parse \"$REV^3:\" 2>/dev/null)\n\nI see yet another rev-parse that you did not modify.\n\n>  }\n>  \n> @@ -531,7 +531,7 @@ drop_stash () {\n>  \t\tdie \"$(eval_gettext \"\\${REV}: Could not drop stash entry\")\"\n>  \n>  \t# clear_stash if we just dropped the last stash entry\n> -\tgit rev-parse --verify \"$ref_stash@{0}\" >/dev/null 2>&1 || clear_stash\n> +\tgit rev-parse --quiet --verify \"$ref_stash@{0}\" || clear_stash\n\nAgain not correct.\n\n>  }\n>  \n>  apply_to_branch () {\n> \n\n-- Hannes\n"},{"id":"248676","messageId":"20140831073550.GA66236@gmail.com","threadId":"37457","inReplyTo":"54023D01.6090504@kdbg.org","subject":"Re: [PATCH 2/2] stash: prefer --quiet over shell redirection","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-08-31T07:35:51Z","receivedAt":"2014-08-31T07:35:51Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, Aug 30, 2014 at 11:07:13PM +0200, Johannes Sixt wrote:\n> Am 30.08.2014 21:30, schrieb David Aguilar:\n> > @@ -392,12 +392,12 @@ parse_flags_and_rev()\n> >  \t\t;;\n> >  \tesac\n> >  \n> > -\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\" 2>/dev/null) || {\n> > +\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\") || {\n> >  \t\treference=\"$1\"\n> >  \t\tdie \"$(eval_gettext \"\\$reference is not valid reference\")\"\n> >  \t}\n> >  \n> > -\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\" 2>/dev/null) &&\n> > +\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\") &&\n> >  \tset -- $(git rev-parse \"$REV\" \"$REV^1\" \"$REV:\" \"$REV^1:\" \"$REV^2:\" 2>/dev/null) &&\n> \n> I see another rev-parse that you did not modify. An omission?\n\nThe docs for --quiet say, \"Only meaningful in --verify mode\", so I didn't touch\nthe non-verify call-sites.\n\nThanks for the review. I'll address your notes and send a v2 shortly.\n-- \nDavid\n"}]}