From: Jeff King <peff@peff.net>
To: felipe@felipegasper.com
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Fix comma splices
Date: Mon, 22 Jan 2018 18:08:20 -0500 [thread overview]
Message-ID: <20180122230819.GB21604@sigill.intra.peff.net> (raw)
In-Reply-To: <5a656638.LthG4qGgtrvPcSKz%felipe@felipegasper.com>
On Sun, Jan 21, 2018 at 10:19:04PM -0600, felipe@felipegasper.com wrote:
> Subject: [PATCH] Fix comma splices in remote.c
> [...]
> @@ -2123,9 +2123,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
> _(" (use \"git push\" to publish your local commits)\n"));
> } else if (!ours) {
> strbuf_addf(sb,
> - Q_("Your branch is behind '%s' by %d commit, "
> + Q_("Your branch is behind '%s' by %d commit "
> "and can be fast-forwarded.\n",
> - "Your branch is behind '%s' by %d commits, "
> + "Your branch is behind '%s' by %d commits "
> "and can be fast-forwarded.\n",
Yes, the original violates the usual English rules for commas, which say
that the dependent clause joined with a conjunction should not have a
comma.
(To be pedantic, these aren't comma splices. A comma splice joins two
independent clauses with a comma and _without_ a conjunction).
This kind of comma _can_ actually be considered correct if it makes the
sentence more clear, or to indicate a more extreme contrast. I tend to
agree with you, though, that it does not help clarity here at all.
> @@ -2134,11 +2134,11 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
> _(" (use \"git pull\" to update your local branch)\n"));
> } else {
> strbuf_addf(sb,
> - Q_("Your branch and '%s' have diverged,\n"
> - "and have %d and %d different commit each, "
> + Q_("Your branch and '%s' have diverged.\n"
> + "They have %d and %d different commit each, "
> "respectively.\n",
This one is the same case as above, but you solved it differently. I
agree that the result reads more clearly than the original. And more
clearly than:
Your branch and '%s' have diverged and have %d and %d different
commits each, respectively.
The first hunk could use the same "start a new sentence" trick, but it's
pretty clear as a single sentence. So I think your patch is doing the
right thing, my pedantic explanations notwithstanding. ;)
Reading this did make me wonder two things, though:
1. Do we really need the newline after "diverged"? Especially with the
comma, it makes the result look funnily wrapped (unless you have an
extremely long upstream branch name).
OTOH, if we switch it to a period as your patch does, I think the
line break looks much more natural.
2. Is this Q_() here actually helping? It triggers on "ours + theirs"
being greater than 1. So the "singular" case could only come up if
you had "0" in one of the cases. But:
...and have 0 and 1 different commit each...
makes no sense to me in English. It would still be "commits". I
could accept:
...and have 1 commit each...
for the case where it's 1/1. But we would use "1 and 1 different
commits each" (which is correct, albeit slightly clunkier).
In fact, I don't think the singular case here can _ever_ trigger,
because this is the "else" block we get to when we know that
neither is zero. So I think the singular half of this Q_() could
just go away.
Like:
diff --git a/remote.c b/remote.c
index 4c84ba88dc..52672ac658 100644
--- a/remote.c
+++ b/remote.c
@@ -2096,13 +2096,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
_(" (use \"git pull\" to update your local branch)\n"));
} else {
strbuf_addf(sb,
- Q_("Your branch and '%s' have diverged,\n"
- "and have %d and %d different commit each, "
- "respectively.\n",
- "Your branch and '%s' have diverged,\n"
- "and have %d and %d different commits each, "
- "respectively.\n",
- ours + theirs),
+ _("Your branch and '%s' have diverged,\n"
+ "and have %d and %d different commits each, "
+ "respectively.\n"),
base, ours, theirs);
if (advice_status_hints)
strbuf_addstr(sb,
(if I were doing such a series, I'd probably do that first as a
preparatory step, and then do grammatical fix on top).
-Peff
next prev parent reply other threads:[~2018-01-22 23:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-22 4:19 [PATCH] Fix comma splices felipe
2018-01-22 23:08 ` Jeff King [this message]
2018-01-22 23:23 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180122230819.GB21604@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=felipe@felipegasper.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).