{"thread":{"id":"47663","subject":"[PATCH] Fix comma splices","startedAt":"2018-01-22T04:35:16Z","lastAt":"2018-01-22T23:23:07Z","messageCount":3,"participants":["felipe@felipegasper.com","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"337047","messageId":"5a656638.LthG4qGgtrvPcSKz%felipe@felipegasper.com","threadId":"47663","inReplyTo":null,"subject":"[PATCH] Fix comma splices","fromName":"","fromEmail":"felipe@felipegasper.com","sentAt":"2018-01-22T04:19:04Z","receivedAt":"2018-01-22T04:35:16Z","isPatch":true,"sender":{"key":"felipe@felipegasper.com","avatar":null},"body":"From b18bd6babc2d6a6f79177acfa2416bb5bf2b153f Mon Sep 17 00:00:00 2001\nFrom: Felipe Gasper \nDate: Sun, 21 Jan 2018 23:01:47 -0500\nSubject: [PATCH] Fix comma splices in remote.c\n\nSigned-off-by: Felipe Gasper \n---\n remote.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 4e93753e1..e8ba808c5 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2123,9 +2123,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t\t_(\" (use \\\"git push\\\" to publish your local commits)\\n\"));\n \t} else if (!ours) {\n \t\tstrbuf_addf(sb,\n-\t\t\tQ_(\"Your branch is behind '%s' by %d commit, \"\n+\t\t\tQ_(\"Your branch is behind '%s' by %d commit \"\n \t\t\t \"and can be fast-forwarded.\\n\",\n-\t\t\t \"Your branch is behind '%s' by %d commits, \"\n+\t\t\t \"Your branch is behind '%s' by %d commits \"\n \t\t\t \"and can be fast-forwarded.\\n\",\n \t\t\t theirs),\n \t\t\tbase, theirs);\n@@ -2134,11 +2134,11 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t\t_(\" (use \\\"git pull\\\" to update your local branch)\\n\"));\n \t} else {\n \t\tstrbuf_addf(sb,\n-\t\t\tQ_(\"Your branch and '%s' have diverged,\\n\"\n-\t\t\t \"and have %d and %d different commit each, \"\n+\t\t\tQ_(\"Your branch and '%s' have diverged.\\n\"\n+\t\t\t \"They have %d and %d different commit each, \"\n \t\t\t \"respectively.\\n\",\n-\t\t\t \"Your branch and '%s' have diverged,\\n\"\n-\t\t\t \"and have %d and %d different commits each, \"\n+\t\t\t \"Your branch and '%s' have diverged.\\n\"\n+\t\t\t \"They have %d and %d different commits each, \"\n \t\t\t \"respectively.\\n\",\n \t\t\t ours + theirs),\n \t\t\tbase, ours, theirs);\n-- \n2.15.1\n\n"},{"id":"337116","messageId":"20180122230819.GB21604@sigill.intra.peff.net","threadId":"47663","inReplyTo":"5a656638.LthG4qGgtrvPcSKz%felipe@felipegasper.com","subject":"Re: [PATCH] Fix comma splices","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-22T23:08:20Z","receivedAt":"2018-01-22T23:08:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 21, 2018 at 10:19:04PM -0600, felipe@felipegasper.com wrote:\n\n> Subject: [PATCH] Fix comma splices in remote.c\n> [...]\n> @@ -2123,9 +2123,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n> \t\t\t\t_(\" (use \\\"git push\\\" to publish your local commits)\\n\"));\n> \t} else if (!ours) {\n> \t\tstrbuf_addf(sb,\n> -\t\t\tQ_(\"Your branch is behind '%s' by %d commit, \"\n> +\t\t\tQ_(\"Your branch is behind '%s' by %d commit \"\n> \t\t\t \"and can be fast-forwarded.\\n\",\n> -\t\t\t \"Your branch is behind '%s' by %d commits, \"\n> +\t\t\t \"Your branch is behind '%s' by %d commits \"\n> \t\t\t \"and can be fast-forwarded.\\n\",\n\nYes, the original violates the usual English rules for commas, which say\nthat the dependent clause joined with a conjunction should not have a\ncomma.\n\n(To be pedantic, these aren't comma splices. A comma splice joins two\nindependent clauses with a comma and _without_ a conjunction).\n\nThis kind of comma _can_ actually be considered correct if it makes the\nsentence more clear, or to indicate a more extreme contrast. I tend to\nagree with you, though, that it does not help clarity here at all.\n\n> @@ -2134,11 +2134,11 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n> \t\t\t\t_(\" (use \\\"git pull\\\" to update your local branch)\\n\"));\n> \t} else {\n> \t\tstrbuf_addf(sb,\n> -\t\t\tQ_(\"Your branch and '%s' have diverged,\\n\"\n> -\t\t\t \"and have %d and %d different commit each, \"\n> +\t\t\tQ_(\"Your branch and '%s' have diverged.\\n\"\n> +\t\t\t \"They have %d and %d different commit each, \"\n> \t\t\t \"respectively.\\n\",\n\nThis one is the same case as above, but you solved it differently. I\nagree that the result reads more clearly than the original. And more\nclearly than:\n\n Your branch and '%s' have diverged and have %d and %d different\n commits each, respectively.\n\nThe first hunk could use the same \"start a new sentence\" trick, but it's\npretty clear as a single sentence. So I think your patch is doing the\nright thing, my pedantic explanations notwithstanding. ;)\n\nReading this did make me wonder two things, though:\n\n 1. Do we really need the newline after \"diverged\"? Especially with the\n comma, it makes the result look funnily wrapped (unless you have an\n extremely long upstream branch name).\n\n OTOH, if we switch it to a period as your patch does, I think the\n line break looks much more natural.\n\n 2. Is this Q_() here actually helping? It triggers on \"ours + theirs\"\n being greater than 1. So the \"singular\" case could only come up if\n you had \"0\" in one of the cases. But:\n\n ...and have 0 and 1 different commit each...\n\n makes no sense to me in English. It would still be \"commits\". I\n could accept:\n\n ...and have 1 commit each...\n\n for the case where it's 1/1. But we would use \"1 and 1 different\n commits each\" (which is correct, albeit slightly clunkier).\n\n In fact, I don't think the singular case here can _ever_ trigger,\n because this is the \"else\" block we get to when we know that\n neither is zero. So I think the singular half of this Q_() could\n just go away.\n\n Like:\n\ndiff --git a/remote.c b/remote.c\nindex 4c84ba88dc..52672ac658 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2096,13 +2096,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t\t_(\" (use \\\"git pull\\\" to update your local branch)\\n\"));\n \t} else {\n \t\tstrbuf_addf(sb,\n-\t\t\tQ_(\"Your branch and '%s' have diverged,\\n\"\n-\t\t\t \"and have %d and %d different commit each, \"\n-\t\t\t \"respectively.\\n\",\n-\t\t\t \"Your branch and '%s' have diverged,\\n\"\n-\t\t\t \"and have %d and %d different commits each, \"\n-\t\t\t \"respectively.\\n\",\n-\t\t\t ours + theirs),\n+\t\t\t_(\"Your branch and '%s' have diverged,\\n\"\n+\t\t\t \"and have %d and %d different commits each, \"\n+\t\t\t \"respectively.\\n\"),\n \t\t\tbase, ours, theirs);\n \t\tif (advice_status_hints)\n \t\t\tstrbuf_addstr(sb,\n\n(if I were doing such a series, I'd probably do that first as a\npreparatory step, and then do grammatical fix on top).\n\n-Peff\n"},{"id":"337119","messageId":"xmqq7es9jx63.fsf@gitster.mtv.corp.google.com","threadId":"47663","inReplyTo":"20180122230819.GB21604@sigill.intra.peff.net","subject":"Re: [PATCH] Fix comma splices","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-22T23:23:00Z","receivedAt":"2018-01-22T23:23:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> (To be pedantic, these aren't comma splices. A comma splice joins two\n> independent clauses with a comma and _without_ a conjunction).\n\nThanks for clearing up the \"Huh?\" I felt earlier when I threw the\npatch to \"to look at later\" bin after finding updated text for most\nof them read not much better than the original, at least to me.\n\nSo I am OK to take this patch (or an updated version), but the log\nmessage needs updating, I guess.\n\n> 2. Is this Q_() here actually helping? It triggers on \"ours + theirs\"\n> ...\n> In fact, I don't think the singular case here can _ever_ trigger,\n\nExactly.\n\n"}]}