Gender neutral language

Patch no longer applies.

Rerolled and added some new edits.

I think it would be preferable, when possible, to avoid number disagreement in pronouns ("user" = singular, "their" = plural).

Current: A module developer has to tag their backend service with "backend_overridable":
Proposed: A module developer has to tag the backend service with "backend_overridable":

Current patch: of node_access_rebuild() lets the user perform their changes and actually
Proposed: of node_access_rebuild() lets the user perform changes and actually

and so on...

• Changed plural pronouns ("they","their") with singular referents to singular or avoided.
• Changed "he or she" to "s/he".

• Changed plural pronouns ("they","their") with singular referents to singular or avoided.
• Changed "he or she" to "s/he".
• Made syntax a bit more natural

I'm pleased that this issue exists and that we are working to resolution for it.

I have reviewed the issue and find the rationale and patch to be sound.

At first I wondered if the construction of "s/he" was easily understood by those who understand English but are not native English speakers, but "s/he" is defined at:

http://www.merriam-webster.com/dictionary/s/he
http://www.dictionary.com/browse/s-he?s=t

so I am guessing that it should be fine.

Completely agreed with cleaning this up.

However, in general we use 'they/their' as a singular gender-neutral pronoun rather than he/she or s/he which is quite clunky. I very rarely see he/she in text written in the past 5 years, and while they/their has got more popular recently, it's also been used for decades/centuries in English so the singular/plural ambivalence shouldn't be a problem in practice.

It is also in the style guide for Drupal.org - see https://www.drupal.org/drupalorg/style-guide/words-phrases

Also, this is a living document. It should and will change as life happens. Had we written it ten years ago, it probably wouldn't have recommended using gender-neutral pronouns. Maybe that practice will be so common by the 2018 version of this guide that we don't even have to include it anymore.

Used \s(s?he|him|her)\s

One thing we can't change is the GPL v2 license which says:

it is up to the author/donor to decide if he or she is willing
to distribute software through any other system and a licensee cannot
impose that choice.

This is unnecessarily binary. But we need to file an issue against the GPL.

We also have some test text that uses he or she but is not help text and so can be ignored. I've changed one because ...and she could take it away seems odd and makes me ask why? Also made All the faith he had had had had no effect on the outcome of his life. not gendered because again not sure why.

Whoops

1. +++ b/core/modules/quickedit/js/views/EditorView.es6.js
   @@ -149,7 +149,7 @@
   -            // When the user has indicated he wants to save his changes to this
   +            // When the user has triggered a save to their changes to this
   

   I think this one could do with finessing.

2. +++ b/core/modules/workspaces/tests/src/Functional/WorkspaceBypassTest.php
   @@ -55,8 +55,8 @@ public function testBypassOwnWorkspace() {
   -    // Because editor 2 has the bypass permission, he should be able to create
   -    // and edit any node.
   +    // Because editor 2 has the bypass permission, editor 2 should be able to
   +    // create and edit any node.
   

   This comment is weird because it does not reflect what is actually tested for. Probably should have a followup.

The title caught my eye so had a read of the patch. Sorry, no suggestions for the two point in the previous comment.

1. +++ b/core/modules/block/tests/src/Functional/BlockLanguageTest.php
   @@ -28,7 +28,6 @@ class BlockLanguageTest extends BrowserTestBase {
   -    // Create a new user, allow him to manage the blocks and the languages.
   

   Removing a comment caught my eye. This is in a test setup and not complicated, so it just takes a moment longer to parse the code. That is ok here, I think.

2. +++ b/core/modules/quickedit/js/views/EditorView.es6.js
   @@ -124,7 +124,7 @@
   +            // The user is the process of activating in-place editing: if
   

   s/is/is in/ ?

Discussed #22.1 IRL with @justafish and they pointed out that to their changes is superfluous language.

Fixed #23 too.

I've opened #3021247: Bypass workspace access permission is not working as expected for the workspace test issue.

Reviewed the changes since my last review in #23. The one issue I raised that needed a fix has been fixed, and the two from #22 has been addressed as well. One is fixed and the other has a followup.
This is good to go.

Working on a sniff for coder and have improved the regex to (^|\W)(he|she|him|his|her|hers)($|\W) this results in a new patch.

I've opened #3021632: Add a sniff to find unnecessarily gendered language in code comments so we can automatically check and don't regress.

A comment from @aburke626 on slack prompted me to realise there is also (^|\W)(woman|man|boy|girl)($|\W).

That finds:

 * $options = [
 *   1 => ['first_name' => 'Indy', 'last_name' => 'Jones'],
 *   2 => ['first_name' => 'Darth', 'last_name' => 'Vader'],
 *   3 => ['first_name' => 'Super', 'last_name' => 'Man'],
 * ];

Where I think we could introduce some diversity into our super hero examples.

And it also finds a view with an option

        field_test_list_string_value:
          id: field_test_list_string_value
          table: field_data_field_test_list_string
          field: field_test_list_string_value
          relationship: none
          group_type: group
          admin_label: ''
          operator: or
          value:
            man: man
            woman: woman

which is unnecessarily binary. Given that that is test change I'm going to file a separate issue to address that.

1. +++ b/core/modules/block/tests/src/Functional/BlockLanguageTest.php
   @@ -28,7 +28,6 @@ class BlockLanguageTest extends BrowserTestBase {
   -    // Create a new user, allow him to manage the blocks and the languages.
   

   Nice!

2. +++ b/core/modules/views_ui/views_ui.module
   @@ -318,7 +318,7 @@ function views_ui_views_analyze(ViewExecutable $view) {
      // system, so the alias will not work anymore. Report this to the user,
   -  // because he probably wanted something else.
   +  // because this is probably in error.
      foreach ($view->displayHandlers as $display) {
   

   s/in/an

3. +++ b/core/modules/workspaces/tests/src/Functional/WorkspaceBypassTest.php
   @@ -55,8 +55,8 @@ public function testBypassOwnWorkspace() {
   -    // Because editor 2 has the bypass permission, he should be able to create
   -    // and edit any node.
   +    // Because editor 2 has the bypass permission, editor 2 should be able to
   +    // create and edit any node.
   

   I like how this improves the overall readability.

Thanks for the review @dawehner re #2 - funnily enough something can be in error and something can be an error :) However I've totally rewritten comment because it is unnecessary wordy.

I've also replaced the superheroes. I don't feel that we could ever come up with a list that is truly diverse and have it be useful documentation therefore I've gone for shapes and colours.

I've also checked for guy and guys - not found in our codebase.

Patch provided by @alexpott applies without problems; in the section shown in the image below I'm guessing it doesn't have to be so many had words right?

@Mark Slatinek it's about how many times had can appear in a sentence and it still be a proper english :) see https://english.stackexchange.com/questions/139436/all-the-faith-he-had-...

Ohh okay didn't know that, thank you for the info :)

Ok so now that coder has a rule and we've updated coder in core we can apply the rule to make sure we don't regress. Nice.

It'd be great to get this to rtbc and fixed in 8.8.x because these comments are completely unnecessarily written this way. And whilst our code comments are only a small part of how we welcome people to Drupal development there is no reason for them to unnecessarily exclusive.

I've run phpcs with this patch applied and there are no errors. I added a "He" to a comment in the code base and re-ran and saw:

FILE: ...b/Drupal/Component/DependencyInjection/Dumper/PhpArrayDumper.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 14 | ERROR | Unnecessarily gendered language in a comment
    |       | (Drupal.Commenting.GenderNeutralComment.GenderNeutral)
----------------------------------------------------------------------

Glad to see this cleaned up and with the rule added

Committed 615123f and pushed to 8.8.x. Thanks!