Remove as many "..." and ellipsis characters from the codebase as possible without altering the meaning of text

This is a good code cleanup. I read through the code, looks like an improvement.

1. +++ b/.htaccess
   @@ -89,14 +89,14 @@ DirectoryIndex index.php index.html index.htm
   -  # (http://example.com/... will be redirected to http://www.example.com/...)
   +  # (http://example.com/* will be redirected to http://www.example.com/*)
   ...
   -  # (http://www.example.com/... will be redirected to http://example.com/...)
   +  # (http://www.example.com/* will be redirected to http://example.com/*)
   
   +++ b/web.config
   @@ -35,7 +35,7 @@
   -     http://example.com/... will be redirected to http://www.example.com/...)
   +     http://example.com/foo will be redirected to http://www.example.com/foo)
   
   @@ -48,7 +48,7 @@
   -     http://www.example.com/... will be redirected to http://example.com/...)
   +     http://www.example.com/foo will be redirected to http://example.com/foo)
   

   Consistency?

2. +++ b/core/misc/ajax.js
   @@ -155,7 +155,7 @@
   -        message: Drupal.t('Please wait...')
   +        message: Drupal.t('Please wait.')
   
   +++ b/core/modules/ckeditor/js/ckeditor.js
   @@ -145,7 +145,7 @@
   -      var $content = $('<div class="ckeditor-dialog-loading"><span style="top: -40px;" class="ckeditor-dialog-loading-link"><a>' + Drupal.t('Loading...') + '</a></span></div>');
   +      var $content = $('<div class="ckeditor-dialog-loading"><span style="top: -40px;" class="ckeditor-dialog-loading-link"><a>' + Drupal.t('Loading.') + '</a></span></div>');
   
   +++ b/core/modules/file/tests/file_module_test/file_module_test.module
   @@ -25,7 +25,7 @@ function file_module_test_form($form, &$form_state, $tree = TRUE, $extended = TR
   -    '#progress_message' => t('Please wait...'),
   +    '#progress_message' => t('Please wait.'),
   
   +++ b/core/modules/forum/forum.module
   @@ -24,7 +24,7 @@ function forum_help($route_name, Request $request) {
   -      $output .= '<ul><li>' . t('Threaded comments submitted by users (for example, <em>You have these options...</em>).') . '</li>';
   +      $output .= '<ul><li>' . t('Threaded comments submitted by users (for example, <em>You have these options:</em>).') . '</li>';
   
   +++ b/core/modules/update/src/Controller/UpdateController.php
   @@ -73,7 +73,7 @@ public function updateStatusManually() {
   -      'progress_message' => t('Trying to check available update data ...'),
   +      'progress_message' => t('Trying to check available update data.'),
   
   +++ b/core/modules/update/src/UpdateManager.php
   @@ -174,7 +174,7 @@ public function fetchDataBatch(&$context) {
   -      $context['message'] = $this->t('Checking available update data ...');
   +      $context['message'] = $this->t('Checking available update data.');
   

   Why are we affecting end user text in this issue? When I press submit on this comment Chrome will tell me "Waiting for drupal.org..." - is a full stop really correct in these instances? Shouldn't we be using the utf8 character as suggested in #44987: [meta] Use the UTF-8 ellipsis character instead of "..." in the user interface?

I'm fine with standardizing on http://example.com/foo

The point of this patch is to try to remove ... without the use of an ellipsis. These all look fine to me:

• Please wait.
• Loading.
• Threaded comments submitted by users (for example, You have these options:
• Trying to check available update data.
• Checking available update data.

But ya, I'm not all that particular. I'd seek out someone who has stronger opinions on grammar.

Why is this not in the Documentation component?

Also, if you are going to use "etc." as a replacement it needs to end in . as it is an abbreviation for the Latin word "etcetera".

I also don't think this is a great idea:

- *     // ...
+ *     ...

But in any case, it seems to be out of scope for the issue summary here, which is about replacing ... with other alternatives, and this substitution does not do that.

And in general, I don't think this whole issue is necessary. The issue summary does not (in my opinion) really give any good reasons why ... should not be used. Just because some other languages may not use it (and is that even true?) is not a good reason why it should not be used. We write comments and translatable text in English, and anyone wanting to read or translate it needs to understand English -- that is just how it is. Avoiding particular punctuation or words because some people might not know them is not really viable.

Re-rolled the patch as it no longer applied.

#7.1 Changed .htaccess to use /foo so it's consistent with web.config

#7.2 Reverted them to use '...' we can put in ellipsis in another issue but the purpose here is simplification.

@jhodgdon - I added in the "etc." where the periods were missing. Also put back in the "//" as you requested.

As far as this not being useful, it does seem to clean up a bunch of inconsistencies. It's certainly not critical, but I do think it will make it easier to consistently manage the use of ellipsis in the public interface.

Re-rolled patch in #11.

I read through each change and they make sense to me. Not sure we can hope to get *every* instance in a single patch but this looks good enough for now.

I'm not sure about this change:

@ -1112,13 +1112,13 @@ function _writeHeader($p_filename, $p_stored_filename)
 +++ b/core/lib/Drupal/Core/Archiver/ArchiveTar.php

         // ----- Calculate the checksum
         $v_checksum = 0;
-        // ..... First part of the header
+        // First part of the header.
         for ($i=0; $i<148; $i++)
             $v_checksum += ord(substr($v_binary_data_first,$i,1));
-        // ..... Ignore the checksum value and replace it by ' ' (space)
+        // Ignore the checksum value and replace it by ' ' (space).
         for ($i=148; $i<156; $i++)
             $v_checksum += ord(' ');
-        // ..... Last part of the header
+        // Last part of the header.
         for ($i=156, $j=0; $i<512; $i++, $j++)
             $v_checksum += ord(substr($v_binary_data_last,$j,1));
 

So the first comment in this group has:
// ----- something
and then the others had
// ... continue with the process

I really don't think removing the ... makes the comments more clear?

There are a couple of sections like this in the patch. Why are we doing this?

This change also looks wrong to me:

+++ b/core/modules/field/src/Tests/FormTest.php
-    // TODO : check that non-field inpurs are preserved ('title')...
+    // TODO : check that non-field inputs are preserved ('title').
 

I think it was trying to say "title etc."?

And here:

+++ b/core/modules/field_ui/src/Tests/ManageDisplayTest.php
-    // (for example, unlikely to be part of the "Submitted by ... on ..." line).
+    // (e.g., unlikely to be part of the "Submitted by USER on DATE" line).

Now... I really do not see the point of this issue at all, as I've stated before. But if the point is that some people might not understand ... (in code comments? really????) then please do not also replace "for example" with "e.g.", which about 99% of people do not understand is different from "i.e." (it is misused a LOT). Changing it is out of scope for this issue anyway. Do not do this. Please.

Anyway... I didn't have time to review more of this patch -- I got about half-way through...

But ... Why are we really doing this anyway, in code comments? Code comments are written in English and coders will need to understand English, and no one has even found a language where ... does not exist for the other issue, as far as I know. This just seems like a waste of effort. The code comments being "fixed" here are not actually unclear as they were -- they may not be beautiful prose but they are just in-code comments and I don't really think this effort is necessary. The changes are not really clarifying anything, are they?

So... I'm not marking it as "won't fix", even though that is my opinion, but at a minimum I don't think it should be committed as it is, because I do not think everything in there is an improvement, and at best I think it is kind of a waste of time.

Thanks for your feedback @jhodgdon.. I dropped the fragment for function _writeHeader() & ManageDisplayTest.php, and also took your suggestion to go with "('title'), etc.".

The point of this issue is to where possible remove "..." so that we can more consistently use ellipsis in D8. I'm personally not all that worried about it being in the comments and other places, but heck, if we can clean it up and get rid of a few characters, great....

The point is consistency... That and if we eliminate "..." then we don't have to worry about converting it to an ellipsis.

This is mostly a supporting issue to prepare the way for this issue from 2006 as far as I am concerned #44987: [meta] Use the UTF-8 ellipsis character instead of "..." in the user interface.

We definitely should not be using ... in code comments. They are not ASCII characters. Anyway... as I don't think any of the changes that were previously in the patch actually added to the clarity of the in-line comments, and standards compliance has also not been improved by this patch, I still do not see the point.

Good point about code comments being restricted to ASCII.

here the reroll.

Re-rolled the patch and adding the updated patch.

Thanks for the new patch! There are several problems though:

a) This change in .htaccess doesn't look right to me:

-  # (http://example.com/... will be redirected to http://www.example.com/...)
+  # (http://example.com/foo will be redirected to http://www.example.com/...)

If we change the first ... to foo, we should also change the second ...

And then this other one is also wrong:

-  # (http://www.example.com/... will be redirected to http://example.com/...)
+  # (http://www.example.com/... will be redirected to http://example.com/foo)

In this case if we change the second we should also change the first. Both of these are about redirecting www.somethign to no-www and vice versa.

b) in core/lib/Drupal/Core/Archiver/ArchiveTar.php in _writeHeader() method:

-        // ..... Ignore the checksum value and replace it by ' ' (space)
+      // Ignore the checksum value and replace it by ' ' (space).
         for ($i=148; $i<156; $i++)

Wrong indentation.

c)

+++ b/core/lib/Drupal/Core/Entity/Display/EntityFormDisplayInterface.php
@@ -40,7 +40,7 @@
    *   // widget is 'single-value' (provides the input for one field value,
    *   // most common case), and will therefore be repeated as many times as
    *   // needed, or 'multiple-values' (one single widget allows the input of
-   *   // several values, e.g checkboxes, select box...).
+   *   // several values, e.g checkboxes, select box, etc.).

The punctuaion here is incorrect. If you are going to use "e.g.", it needs to be "... several values; e.g., checkboxes...". Or better yet replace it with "several values; for example, ..."

d) The two hunks in comment.module have the same problem as (c). So does core/modules/entity_reference/src/ConfigurableEntityReferenceItem.php and entity.api.php and taxonomy.module -- and we have a general policy that when fixing up a docs line, we should fix all the problems in that line, so it would be good to fix these.

e)

+++ b/core/modules/field/src/Tests/FormTest.php
@@ -257,7 +257,7 @@ function testFieldFormUnlimited() {
     $this->assertFieldByName("{$field_name}[0][value]", '', 'Widget 1 is displayed');
     $this->assertFieldByName("{$field_name}[1][value]", '', 'New widget is displayed');
     $this->assertNoField("{$field_name}[2][value]", 'No extraneous widget is displayed');
-    // TODO : check that non-field inpurs are preserved ('title')...
+    // TODO : check that non-field inpurs are preserved ('title'), etc.

What is "inpurs", can we change that to 'inputs"? Actually we don't have non-field inputs in 8 do we? This whole comment is suspect.

f)

+++ b/core/modules/update/src/Tests/UpdateCoreTest.php
@@ -277,7 +277,7 @@ function testModulePageSecurityUpdate() {
 
     // Make sure duplicate messages don't appear on Update status pages.
     $this->drupalGet('admin/reports/status');
-    // We're expecting "There is a security update..." inside the status report
+    // We're expecting "There is a security update ..." inside the status report
     // itself, but the drupal_set_message() appears as an li so we can prefix
     // with that and search for the raw HTML.
     $this->assertNoRaw('<li>' . t('There is a security update available for your version of Drupal.'));
@@ -294,7 +294,7 @@ function testModulePageSecurityUpdate() {
    */
   function testServiceUnavailable() {
     $this->refreshUpdateStatus(array(), '503-error');
-    // Ensure that no "Warning: SimpleXMLElement..." parse errors are found.
+    // Ensure that no "Warning: SimpleXMLElement ..." parse errors are found.

Actually, these types of changes, where you add a space before a ..., are not correct. When you put ... in an English sentence, there should not be a space before it.

g)

+++ b/core/modules/views/src/Plugin/views/query/Sql.php
@@ -1403,7 +1403,7 @@ function execute(ViewExecutable $view) {
       $count_query = $count_query->countQuery();
 
       // Add additional arguments as a fake condition.
-      // XXX: this doesn't work... because PDO mandates that all bound arguments
+      // XXX: this doesn't work. because PDO mandates that all bound arguments

That . after work should be a ,

The rest looks OK, but I'm still not sure why we're even doing this?

We're doing this to help close this issue from 2006 #44987: [meta] Use the UTF-8 ellipsis character instead of "..." in the user interface if we don't need to use "..." then we don't need to convert them to proper "…".

This isn't a functional problem, but it would be nice to use the proper UTF-8 ellipsis character more often in Drupal 8.

Thanks, actually I see that information is in the issue summary. Anyway, patch still needs a bit of work...

jhodgdon: Made the changes you referred to

Thanks @ameenkhan07 and welcome to Drupal!

If you set the status to "Needs review" the testbot will run and test the patch for you.

just a re-roll.

This is mostly OK... A few things still to fix:

1. +++ b/core/lib/Drupal/Component/Utility/NestedArray.php
   index ac54bde..6c3b061 100644
   --- a/core/lib/Drupal/Core/Archiver/ArchiveTar.php
   

   I do not know if we are really maintaining ArchiveTar. This class has ***many*** problems with coding standards, and I think we either need to overhaul it completely (in another issue) or leave it alone.

2. +++ b/core/lib/Drupal/Core/Asset/CssOptimizer.php
   @@ -142,7 +142,7 @@ protected function loadNestedFile($matches) {
   +    // to normalize absolute paths here (i.e. remove folder/* segments)
   

   The punctuation of i.e. is not correct here. It needs a comma after i.e. or better yet just remove it because it doesn't add anything to the comment really.

3. +++ b/core/modules/system/entity.api.php
   @@ -1606,7 +1606,7 @@ function hook_ENTITY_TYPE_prepare_form(\Drupal\Core\Entity\EntityInterface $enti
   + *   - form_mode: The form mode; for example, 'default', 'profile', 'register', etc.
   

   This line goes over 80 characters.

4. +++ b/core/modules/update/src/Tests/UpdateCoreTest.php
   @@ -277,7 +277,7 @@ function testModulePageSecurityUpdate() {
   -    // We're expecting "There is a security update..." inside the status report
   +    // We're expecting "There is a security update ..." inside the status report
        // itself, but the drupal_set_message() appears as an li so we can prefix
   

   This is incorrect. When ... follows a word, there should not be a space before it.

1. NP on ArchiveTar.php, we don't want to muck with it. Not sure how this is related to NestedArray.php

2. Ok, I've removed "(i.e. remove folder/* segments)"

3. Thanks.

4. Ok. NP.

OK, one more thing that either changed recently or I totally missed in previous reviews (if so, sorry):

+++ b/core/modules/comment/comment.module
@@ -225,7 +225,7 @@ function comment_node_view_alter(array &$build, EntityInterface $node, EntityVie
  * @param \Drupal\comment\CommentInterface $comment
  *   The comment object.
  * @param $view_mode
- *   (optional) View mode, e.g. 'full', 'teaser'... Defaults to 'full'.
+ *   (optional) View mode; for example, e.g. 'full', 'teaser', etc. Defaults to 'full'.

We should not have both for example and e.g. in this line? ?!?

Also I don't get why we're removing e.g. and replacing with "for example" in some places and not others. It is either out of scope for this issue (in which case don't do it anywhere) or we should be consistent and do it everywhere?

Ok, fixed the "example, e.g." redundancy. Also fixed a change to the core/modules/user/user.module file so that the patch would still apply.

I also standardized on "e.g.,". If someone wants to change it to "for example", we can make that a new issue.

woops.. Here's the patch.

Looks good now, except in user.module:

+ * @param string|null $langcode 

Patch adds an extra space to the end of this line.

You may want to check the rest of the patch for similar... I didn't see anything but may not have checked it all. I only noticed this one because the patch showed this line changed, and I wasn't sure why.

Sorry I missed that.. Just a re-roll sans-spaces.

Oh ya. Noticed this a missing "." - in
+ // Doesn't display SUM, COUNT, MAX, etc. functions in SELECT statement

Worth adding it in this patch?

OK, one more pass...

1. +++ b/core/lib/Drupal/Core/Field/WidgetInterface.php
   @@ -66,7 +66,7 @@ public function settingsSummary();
   -   * #value_callback...) used by the widget do not have access to the original
   +   * #value_callback, etc.) used by the widget do not have access to the original
       * $field_definition passed to the widget's constructor. Therefore, if any
   

   This makes the line go over 80 characters. Needs rewrap.

2. +++ b/core/modules/views/src/Tests/QueryGroupByTest.php
   @@ -183,7 +183,7 @@ public function testGroupByNone() {
   -    // Doesn't display SUM, COUNT, MAX... functions in SELECT statement
   +    // Doesn't display SUM, COUNT, MAX, etc. functions in SELECT statement
    
   

   Yes, if you're fixing this line of comment anyway, might as well add a . at the end. Actually the beginning of this line should not be capitalized either, because it's part of the previous line's sentence.

Ok, here's a reroll. hook_field_widget[_WIDGET_TYPE]_form_alter() is just so long. I wasn't sure if I should continue this path in WidgetInterface.php.

Added the . & fixed the capitalization in #45.2

Thanks! Still one problem:

+++ b/core/lib/Drupal/Core/Field/WidgetInterface.php
@@ -66,10 +66,10 @@ public function settingsSummary();
+   * widget implementing this method, or a hook_field_widget[_WIDGET_TYPE]_form_alter()

This line goes over 80 characters still. The function name needs to be moved to the next line and the rest of the paragraph needs to be rewrapped.

Fixes as per #46.

Thanks everyone! This looks good now.

Added line about beta eval to issue summary (this is only comments/docs).

This only conflicts with #2496053: Rename xmlhttprequest to jqXHR in Drupal Ajax JS in the rtbc queue. Therefore going to commit this now rather than having it hang around.

Committed 23e7b47 and pushed to 8.0.x. Thanks!

1. +++ b/core/modules/system/entity.api.php
   @@ -1625,7 +1625,8 @@ function hook_ENTITY_TYPE_prepare_form(\Drupal\Core\Entity\EntityInterface $enti
   - *   - form_mode: The form mode, e.g. 'default', 'profile', 'register'...
   + *   - form_mode: The form mode; e.g., 'default', 'profile', 'register',
   + *     etc.
   

   Reflowed the comment on commit.

2. +++ b/core/modules/update/src/Tests/UpdateContribTest.php
   @@ -342,7 +342,7 @@ function testUpdateBrokenFetchURL() {
        // We need to check that this string is found as part of a project row,
   -    // not just in the "Failed to get available update data for ..." message
   +    // not just in the "Failed to get available update data" message
        // at the top of the page.
   

   Reflowed the comment on commit.