{"thread":{"id":"34450","subject":"[PATCH] request-pull: improve error message for invalid revision args","startedAt":"2013-07-16T10:46:48Z","lastAt":"2013-07-17T17:28:11Z","messageCount":3,"participants":["Dirk Wallenstein","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"223523","messageId":"20130716104648.GA13275@bottich","threadId":"34450","inReplyTo":null,"subject":"[PATCH] request-pull: improve error message for invalid revision args","fromName":"Dirk Wallenstein","fromEmail":"halsmit@t-online.de","sentAt":"2013-07-16T10:46:48Z","receivedAt":"2013-07-16T10:46:48Z","isPatch":true,"sender":{"key":"halsmit@t-online.de","avatar":null},"body":"When an invalid revision is specified, the error message is:\n\n    fatal: Needed a single revision\n\nThis is misleading because, you might think there is something wrong\nwith the command line as a whole.\n\nNow the user gets a more meaningful error message, showing the invalid\nrevision.\n\nSigned-off-by: Dirk Wallenstein <halsmit@t-online.de>\n---\n\nNotes:\n    I assume, it is not worth the trouble to even try to change the message from\n    rev-parse for this.  People might parse the messages, which is probably why\n    this message still exists.\n\n git-request-pull.sh | 14 ++++++++++++--\n 1 file changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex d566015..f38f0f9 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -51,8 +51,18 @@ fi\n tag_name=$(git describe --exact \"$head^0\" 2>/dev/null)\n \n test -n \"$base\" && test -n \"$url\" || usage\n-baserev=$(git rev-parse --verify \"$base\"^0) &&\n-headrev=$(git rev-parse --verify \"$head\"^0) || exit\n+\n+baserev=$(git rev-parse --verify \"$base\"^0 2>/dev/null)\n+if test -z \"$baserev\"\n+then\n+    die \"fatal: Not a valid revision: $base\"\n+fi\n+\n+headrev=$(git rev-parse --verify \"$head\"^0 2>/dev/null)\n+if test -z \"$headrev\"\n+then\n+    die \"fatal: Not a valid revision: $head\"\n+fi\n \n merge_base=$(git merge-base $baserev $headrev) ||\n die \"fatal: No commits in common between $base and $head\"\n-- \n1.8.3.2.51.g8658a4c\n"},{"id":"223594","messageId":"7vr4ex6rqq.fsf@alter.siamese.dyndns.org","threadId":"34450","inReplyTo":"20130716104648.GA13275@bottich","subject":"Re: [PATCH] request-pull: improve error message for invalid revision args","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-17T17:06:21Z","receivedAt":"2013-07-17T17:06:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Wallenstein <halsmit@t-online.de> writes:\n\n> When an invalid revision is specified, the error message is:\n>\n>     fatal: Needed a single revision\n>\n> This is misleading because, you might think there is something wrong\n> with the command line as a whole.\n>\n> Now the user gets a more meaningful error message, showing the invalid\n> revision.\n>\n> Signed-off-by: Dirk Wallenstein <halsmit@t-online.de>\n> ---\n>\n> Notes:\n>     I assume, it is not worth the trouble to even try to change the message from\n>     rev-parse for this.  People might parse the messages, which is probably why\n>     this message still exists.\n\nYou are right---such a change will break existing scripts, so it is\nnot just \"not worth the trouble\" but is actively wrong to change the\nerror message.\n\n>  git-request-pull.sh | 14 ++++++++++++--\n>  1 file changed, 12 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-request-pull.sh b/git-request-pull.sh\n> index d566015..f38f0f9 100755\n> --- a/git-request-pull.sh\n> +++ b/git-request-pull.sh\n> @@ -51,8 +51,18 @@ fi\n>  tag_name=$(git describe --exact \"$head^0\" 2>/dev/null)\n>  \n>  test -n \"$base\" && test -n \"$url\" || usage\n> -baserev=$(git rev-parse --verify \"$base\"^0) &&\n> -headrev=$(git rev-parse --verify \"$head\"^0) || exit\n> +\n> +baserev=$(git rev-parse --verify \"$base\"^0 2>/dev/null)\n\nUse \"--quiet\" instead?\n\n> +if test -z \"$baserev\"\n> +then\n> +    die \"fatal: Not a valid revision: $base\"\n> +fi\n> +\n> +headrev=$(git rev-parse --verify \"$head\"^0 2>/dev/null)\n> +if test -z \"$headrev\"\n> +then\n> +    die \"fatal: Not a valid revision: $head\"\n> +fi\n>  \n>  merge_base=$(git merge-base $baserev $headrev) ||\n>  die \"fatal: No commits in common between $base and $head\"\n"},{"id":"223598","messageId":"20130717172811.GA12981@bottich","threadId":"34450","inReplyTo":"7vr4ex6rqq.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] request-pull: improve error message for invalid revision args","fromName":"Dirk Wallenstein","fromEmail":"halsmit@t-online.de","sentAt":"2013-07-17T17:28:11Z","receivedAt":"2013-07-17T17:28:11Z","isPatch":true,"sender":{"key":"halsmit@t-online.de","avatar":null},"body":"Currently, when an invalid revision is specified, the error message is:\n\n    fatal: Needed a single revision\n\nThis is misleading because, you might think there is something wrong\nwith the command line as a whole.\n\nNow the user gets a more meaningful error message, showing the invalid\nrevision.\n\nSigned-off-by: Dirk Wallenstein <halsmit@t-online.de>\n---\n\nOn Wed, Jul 17, 2013 at 10:06:21AM -0700, Junio C Hamano wrote:\n> Dirk Wallenstein <halsmit@t-online.de> writes:\n> > +baserev=$(git rev-parse --verify \"$base\"^0 2>/dev/null)\n> \n> Use \"--quiet\" instead?\nOh, of course.\n\n git-request-pull.sh | 14 ++++++++++++--\n 1 file changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex d566015..ebf1269 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -51,8 +51,18 @@ fi\n tag_name=$(git describe --exact \"$head^0\" 2>/dev/null)\n \n test -n \"$base\" && test -n \"$url\" || usage\n-baserev=$(git rev-parse --verify \"$base\"^0) &&\n-headrev=$(git rev-parse --verify \"$head\"^0) || exit\n+\n+baserev=$(git rev-parse --verify --quiet \"$base\"^0)\n+if test -z \"$baserev\"\n+then\n+    die \"fatal: Not a valid revision: $base\"\n+fi\n+\n+headrev=$(git rev-parse --verify --quiet \"$head\"^0)\n+if test -z \"$headrev\"\n+then\n+    die \"fatal: Not a valid revision: $head\"\n+fi\n \n merge_base=$(git merge-base $baserev $headrev) ||\n die \"fatal: No commits in common between $base and $head\"\n-- \n1.8.3.3.2.g85103ba\n"}]}