Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Components][Form] Grammar improvement #4326

Merged
merged 1 commit into from
Nov 8, 2014
Merged

[Components][Form] Grammar improvement #4326

merged 1 commit into from
Nov 8, 2014

Conversation

fabschurt
Copy link

Q A
Doc fix? yes
New docs? no
Applies to 2.3+
Fixed tickets n/a

I think the "each the" form is not correct in English, as "each" must
generally be used as in "each one of", or "each of", forms that should
not be used here either as the countable aspect is already expressed
by "each field" further in the phrase anyway.

@wouterj
Copy link
Member

wouterj commented Oct 15, 2014

"each" and "every" can be used in cases like this afaik. Let's see how our natives think about it

@xabbuh
Copy link
Member

xabbuh commented Oct 15, 2014

@wouterj Yes, but then you have to omit "the".

@fabschurt
Copy link
Author

@xabbuh Yep. So basically it would be:

  • either "that help you render the HTML widget, label and error for each field";
  • or "that help you render each HTML widget, label and error for each field".

@wouterj
Copy link
Member

wouterj commented Oct 31, 2014

@fabschurt I'm in favor of the last option. Can you please update this PR?

@fabschurt
Copy link
Author

@wouterj Done ;-) I did a git push --force in order to keep only 1 commit for the PR, I don't know if it's the best way to go?

@xabbuh
Copy link
Member

xabbuh commented Oct 31, 2014

👍 Nice change.

@fabschurt Squashing commits is not necessary, but we are happy if you do it anyway. :)

@@ -173,7 +173,7 @@ line to your ``composer.json`` file:
}

The TwigBridge integration provides you with several :doc:`Twig Functions </reference/forms/twig_reference>`
that help you render each the HTML widget, label and error for each field
that help you render each HTML widget, label and error for each field
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that it was wrong before, but now it's still a bit odd - we have each HTML... for each field. What would be perfect is actually removing the first each and leaving the:

that help you render the HTML widget, label and error for each field

Thanks for your attention to this! It definitely sounds plain wrong the way it is currently :).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my fault. He did it this way and I suggested something wrong, I missed the second for each somehow...

I think the "each the" form is not correct in English, as "each" must
generally be used as in "each one of", or "each of".
@fabschurt
Copy link
Author

Heh heh, this kind of things is trickier than it sounds :-) I updated the PR, we should be good to go now.

@wouterj
Copy link
Member

wouterj commented Nov 8, 2014

Thanks for you patience, Fabien! The docs got a bit better after this PR :)

@wouterj wouterj merged commit cefddae into symfony:2.3 Nov 8, 2014
wouterj added a commit that referenced this pull request Nov 8, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

[Components][Form] Grammar improvement

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets | n/a

I think the "each the" form is not correct in English, as "each" must
generally be used as in "each one of", or "each of", forms that should
not be used here either as the countable aspect is already expressed
by "each field" further in the phrase anyway.

Commits
-------

cefddae Apply a grammatical improvement
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants