{"thread":{"id":"35929","subject":"[PATCH] demonstrate git-commit --dry-run exit code behaviour","startedAt":"2014-02-21T19:16:54Z","lastAt":"2014-02-24T17:16:01Z","messageCount":4,"participants":["Tay Ray Chuan","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235154","messageId":"1393010214-32306-1-git-send-email-rctay89@gmail.com","threadId":"35929","inReplyTo":null,"subject":"[PATCH] demonstrate git-commit --dry-run exit code behaviour","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2014-02-21T19:16:54Z","receivedAt":"2014-02-21T19:16:54Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"In particular, show that --short and --porcelain, while implying\n--dry-run, do not return the same exit code as --dry-run. This is due to\nthe wt_status.commitable flag being set only when a long status is\nrequested.\n\nNo fix is provided here; with [1], it should be trivial to fix though -\njust a matter of calling wt_status_mark_commitable().\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/242489\n\nSigned-off-by: Tay Ray Chuan \n---\n t/t7501-commit.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 94eec83..d58b097 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -61,11 +61,47 @@ test_expect_success 'nothing to commit' '\n \ttest_must_fail git commit -m initial\n '\n \n+test_expect_success '--dry-run fails with nothing to commit' '\n+\ttest_must_fail git commit -m initial --dry-run\n+'\n+\n+test_expect_success '--short fails with nothing to commit' '\n+\ttest_must_fail git commit -m initial --short\n+'\n+\n+test_expect_success '--porcelain fails with nothing to commit' '\n+\ttest_must_fail git commit -m initial --porcelain\n+'\n+\n+test_expect_success '--long fails with nothing to commit' '\n+\ttest_must_fail git commit -m initial --long\n+'\n+\n test_expect_success 'setup: non-initial commit' '\n \techo bongo bongo bongo >file &&\n \tgit commit -m next -a\n '\n \n+test_expect_success '--dry-run with stuff to commit returns ok' '\n+\techo bongo bongo bongo >>file &&\n+\tgit commit -m next -a --dry-run\n+'\n+\n+test_expect_failure '--short with stuff to commit returns ok' '\n+\techo bongo bongo bongo >>file &&\n+\tgit commit -m next -a --short\n+'\n+\n+test_expect_failure '--porcelain with stuff to commit returns ok' '\n+\techo bongo bongo bongo >>file &&\n+\tgit commit -m next -a --porcelain\n+'\n+\n+test_expect_success '--long with stuff to commit returns ok' '\n+\techo bongo bongo bongo >>file &&\n+\tgit commit -m next -a --long\n+'\n+\n test_expect_success 'commit message from non-existing file' '\n \techo more bongo: bongo bongo bongo bongo >file &&\n \ttest_must_fail git commit -F gah -a\n-- \n1.9.0.291.g027825b\n"},{"id":"235155","messageId":"xmqqa9dk43pi.fsf@gitster.dls.corp.google.com","threadId":"35929","inReplyTo":"1393010214-32306-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH] demonstrate git-commit --dry-run exit code behaviour","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-21T20:21:13Z","receivedAt":"2014-02-21T20:21:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan writes:\n\n> In particular, show that --short and --porcelain, while implying\n> --dry-run, do not return the same exit code as --dry-run. This is due to\n> the wt_status.commitable flag being set only when a long status is\n> requested.\n\nI am not sure if --short/--porcelain should even be accepted by \"git\ncommit\" in the first place. It used to be that \"git status\" and\n\"git commit\" were the same program in a different guise and \"git\nstatus \" were merely a \"git commit --dry-run \",\nbut the recent push is in the direction of making them totally\nseparate in the end-user's minds. So if we want a proper fix, I\nwould actually think that these options should *error out* at the\ncommand line parser level, way before checking if there is anything\nto commit.\n\n> No fix is provided here; with [1], it should be trivial to fix though -\n> just a matter of calling wt_status_mark_commitable().\n>\n> [1] http://article.gmane.org/gmane.comp.version-control.git/242489\n>\n> Signed-off-by: Tay Ray Chuan \n> ---\n> t/t7501-commit.sh | 36 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 36 insertions(+)\n>\n> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\n> index 94eec83..d58b097 100755\n> --- a/t/t7501-commit.sh\n> +++ b/t/t7501-commit.sh\n> @@ -61,11 +61,47 @@ test_expect_success 'nothing to commit' '\n> \ttest_must_fail git commit -m initial\n> '\n> \n> +test_expect_success '--dry-run fails with nothing to commit' '\n> +\ttest_must_fail git commit -m initial --dry-run\n> +'\n> +\n> +test_expect_success '--short fails with nothing to commit' '\n> +\ttest_must_fail git commit -m initial --short\n> +'\n> +\n> +test_expect_success '--porcelain fails with nothing to commit' '\n> +\ttest_must_fail git commit -m initial --porcelain\n> +'\n> +\n> +test_expect_success '--long fails with nothing to commit' '\n> +\ttest_must_fail git commit -m initial --long\n> +'\n> +\n> test_expect_success 'setup: non-initial commit' '\n> \techo bongo bongo bongo >file &&\n> \tgit commit -m next -a\n> '\n> \n> +test_expect_success '--dry-run with stuff to commit returns ok' '\n> +\techo bongo bongo bongo >>file &&\n> +\tgit commit -m next -a --dry-run\n> +'\n> +\n> +test_expect_failure '--short with stuff to commit returns ok' '\n> +\techo bongo bongo bongo >>file &&\n> +\tgit commit -m next -a --short\n> +'\n> +\n> +test_expect_failure '--porcelain with stuff to commit returns ok' '\n> +\techo bongo bongo bongo >>file &&\n> +\tgit commit -m next -a --porcelain\n> +'\n> +\n> +test_expect_success '--long with stuff to commit returns ok' '\n> +\techo bongo bongo bongo >>file &&\n> +\tgit commit -m next -a --long\n> +'\n> +\n> test_expect_success 'commit message from non-existing file' '\n> \techo more bongo: bongo bongo bongo bongo >file &&\n> \ttest_must_fail git commit -F gah -a\n"},{"id":"235171","messageId":"20140222083423.GF1576@sigill.intra.peff.net","threadId":"35929","inReplyTo":"xmqqa9dk43pi.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] demonstrate git-commit --dry-run exit code behaviour","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-22T08:34:23Z","receivedAt":"2014-02-22T08:34:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 21, 2014 at 12:21:13PM -0800, Junio C Hamano wrote:\n\n> Tay Ray Chuan writes:\n> \n> > In particular, show that --short and --porcelain, while implying\n> > --dry-run, do not return the same exit code as --dry-run. This is due to\n> > the wt_status.commitable flag being set only when a long status is\n> > requested.\n> \n> I am not sure if --short/--porcelain should even be accepted by \"git\n> commit\" in the first place. It used to be that \"git status\" and\n> \"git commit\" were the same program in a different guise and \"git\n> status \" were merely a \"git commit --dry-run \",\n> but the recent push is in the direction of making them totally\n> separate in the end-user's minds. So if we want a proper fix, I\n> would actually think that these options should *error out* at the\n> command line parser level, way before checking if there is anything\n> to commit.\n\nI do not think they are any less useful than \"git commit --dry-run\" in\nthe first place. If you want to ask \"what would happen if I ran commit\nwith these arguments\", you can get the answer in any of several formats\n(and --porcelain is the only machine-readable one).\n\nI have never found \"commit --dry-run\" to be useful, but I assumed that\nsomebody does.\n\n-Peff\n"},{"id":"235267","messageId":"xmqqha7obfe6.fsf@gitster.dls.corp.google.com","threadId":"35929","inReplyTo":"20140222083423.GF1576@sigill.intra.peff.net","subject":"Re: [PATCH] demonstrate git-commit --dry-run exit code behaviour","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-24T17:16:01Z","receivedAt":"2014-02-24T17:16:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Feb 21, 2014 at 12:21:13PM -0800, Junio C Hamano wrote:\n>\n>> Tay Ray Chuan writes:\n>> \n>> > In particular, show that --short and --porcelain, while implying\n>> > --dry-run, do not return the same exit code as --dry-run. This is due to\n>> > the wt_status.commitable flag being set only when a long status is\n>> > requested.\n>> \n>> I am not sure if --short/--porcelain should even be accepted by \"git\n>> commit\" in the first place. It used to be that \"git status\" and\n>> \"git commit\" were the same program in a different guise and \"git\n>> status \" were merely a \"git commit --dry-run \",\n>> but the recent push is in the direction of making them totally\n>> separate in the end-user's minds. So if we want a proper fix, I\n>> would actually think that these options should *error out* at the\n>> command line parser level, way before checking if there is anything\n>> to commit.\n>\n> I do not think they are any less useful than \"git commit --dry-run\" in\n> the first place. If you want to ask \"what would happen if I ran commit\n> with these arguments\", you can get the answer in any of several formats\n> (and --porcelain is the only machine-readable one).\n\nHmph.\n\n> I have never found \"commit --dry-run\" to be useful, but I assumed that\n> somebody does.\n\nSame here, and I did not really consider \"commit --short\" was\nintentionally a valid short-hand for \"commit --dry-run --short\", but\nits working as such was an accident, hence my comment.\n"}]}