The proper use of 'e.g.' and 'i.e.' are confusing and reportedly are one of the most frequent sources of mistakes editors see in technical writing/documentation. Drupal's docblock and code documentation includes some examples of incorrect usage as well.

Let's start to correct some of that confusion by replacing 'e.g.,' by what it means in English: 'for instance,'. This will require a number of patches until 'e.g.' is appropriately replaced. Feedback on patch size for ease of review and commit processing would be appreciated.

Note: Frequently, the replace with 'for instance' requires the re-wrapping of docblocks and code comments to be near but less than 80 characters.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lars Toomre created an issue. See original summary.

Lars Toomre’s picture

Here is an initial patch converting instances of 'e.g.' in thirty-four files. It does not convert any comments in *.js files nor does it touch any user facing strings.

Is this too big, about right or two small? if all of the patches for this conversion were this size, it would take about ten commits to eliminate the use of e.g. in docblock and code documentation.

jhodgdon’s picture

Status: Active » Needs work

Regarding patch size, I wouldn't make this type of patch any bigger than it is. I think it's about the right size.

And it's mostly wonderful! Thanks! A few small things to address...

  1. +++ b/core/CHANGELOG.txt
    @@ -813,13 +813,13 @@ Drupal 4.7.0, 2006-05-01
    -    * Added advanced search operators (e.g., phrase, node type, etc.).
    +    * Added advanced search operators (for example, phrase, node type, etc.).
    

    In this case, I think we could just eliminate the e.g., since there is an etc. at the end of the list?

  2. +++ b/core/includes/database.inc
    @@ -29,14 +29,14 @@
    + *   an array of values to be expanded; for example, with an IN query, the
    

    The punctuation here is incorrect. ; means "here's another clause of the sentence", but this isn't another clause.

    Really the "for example, with an IN query" needs to I think be in ().

  3. +++ b/core/lib/Drupal.php
    @@ -420,34 +420,34 @@ public static function httpClient() {
    +   *   'OR' if any of them is sufficient.  Defaults to 'AND'.
    

    Extra space after .

  4. +++ b/core/lib/Drupal.php
    @@ -420,34 +420,34 @@ public static function httpClient() {
    +   *   'OR' if any of them is sufficient.  Defaults to 'AND'.
    

    Extra space after .

  5. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
    @@ -33,16 +33,17 @@ class UrlHelper {
    +   *   nested items. Defaults to any empty string.
    

    any => an

  6. +++ b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php
    @@ -139,17 +140,17 @@ public function convertTokensToKeys(array $context_tokens) {
    -   * Examples — remember that the period indicates hierarchy and the colon can
    -   * be used to get a specific value of a calculated cache context:
    +   * Examples: Remember that the period indicates hierarchy and the colon can be
    +   * used to get a specific value of a calculated cache context:
    

    Hm. I don't think I like having two : in one sentence, perhaps leave these two lines as they were?

  7. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorageInterface.php
    @@ -15,16 +15,16 @@
    +   *   The full configuration name to extract the ID from. For example,
        *   'views.view.archive'.
    

    Probably the . should be a ; here?

  8. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorageInterface.php
    @@ -15,16 +15,16 @@
    +   *   The config prefix of the configuration entity. For example, 'views.view'.
    

    and here

  9. +++ b/core/lib/Drupal/Core/Config/InstallStorage.php
    @@ -77,15 +77,15 @@ public function __construct($directory = self::CONFIG_INSTALL_DIRECTORY, $collec
    +   *   searched in the profile first (whereas the profile is never the owner),
    +   *   only afterwards check for a corresponding module or theme.
    

    If we're fixing this up, we could also fix the comma splice (the last comma should be a ; ).

  10. +++ b/core/lib/Drupal/Core/Image/ImageInterface.php
    @@ -83,15 +83,16 @@ public function getToolkitId();
    +   *   "['width' => 50, 'height' => 100, 'upscale' => TRUE]". Defaults for an
    

    defaults for => defaults to

    And why did you put the array in quotes? That makes no sense to me. Better to put in @code ... @endcode.

  11. +++ b/core/lib/Drupal/Core/Image/ImageInterface.php
    @@ -117,14 +118,14 @@ public function save($destination = NULL);
    +   *   (Optional) The extension of the image file (for instance, 'png', 'gif',
    

    (Optional) should be lower-case

  12. +++ b/core/lib/Drupal/Core/Image/ImageInterface.php
    @@ -228,16 +229,16 @@ public function desaturate();
    +   *   0x000000 for black, 0xff00ff for magenta, and 0xffffff for white. For
    +   *   images that support transparency, this will default to transparent;
    +   *   otherwise, it will be white when NULL (the default) is specified.
    

    This last sentence rewrite was a bit confusing for me to read. I think maybe starting it with "When NULL (the default) is specified," might make it clearer that both the white and transparent mentions are for the default case? Because as it is now, the "the default" bit kind of looks like it's just for the "otherwise" clause.

  13. +++ b/core/lib/Drupal/Core/Render/RendererInterface.php
    @@ -135,14 +135,14 @@ public function renderPlaceholder($placeholder, array $elements);
    +   *       converted to a final value depending on the request. (for instance,
    +   *       'user' is mapped to the current user's ID.)
    

    For instance needs capitalization.

  14. +++ b/core/lib/Drupal/Core/StreamWrapper/StreamWrapperInterface.php
    @@ -63,14 +63,14 @@
       /**
    -   * Not visible in the UI or accessible via web, but readable and writable.
    -   * E.g. the temporary directory for uploads.
    +   * Not visible in the UI or accessible via web, but readable and writable; for
    +   * instance, the temporary directory for uploads.
        */
       const HIDDEN = 0x000C;
    

    If we're fixing up this doc block, could we have a one-line summary to start it off?

  15. +++ b/core/lib/Drupal/Core/TypedData/TypedDataManagerInterface.php
    @@ -200,15 +201,16 @@ public function getValidationConstraintManager();
    +   * required. Besides that, any constraints defined for the data type (i.e.,
    

    Since it's in the same paragraph, want to also fix up the i.e. in this line?

  16. +++ b/core/lib/Drupal/Core/TypedData/TypedDataManagerInterface.php
    @@ -219,18 +221,18 @@ public function getDefaultConstraints(DataDefinitionInterface $definition);
    +   * unified adapter wrapping a primary value (for example, a string, an entity,
    +   * ...) which is the canonical representation that consuming code like
    

    In a previous issue, we got rid of ... in favor of etc. This one must have crept back in... also if you have ... or etc. you probably do not need "for example" in there also!

  17. +++ b/core/modules/ckeditor/src/Plugin/CKEditorPlugin/Internal.php
    @@ -474,17 +474,17 @@ protected function generateACFSettings(Editor $editor) {
    +          // it will still be allowed by CKEditor for instance, to define any ¶
    

    trailing space on this line.

  18. +++ b/core/modules/file/src/Tests/FileManagedTestBase.php
    @@ -31,14 +31,14 @@ protected function setUp() {
    +   *   Array with string containing with the hook name; for example, 'load',
    

    Um. This line doesn't make any sense? Array with string containing with ?

Lars Toomre’s picture

Status: Needs work » Needs review
FileSize
17.88 KB
87.47 KB

Re 3.2 - Sorry about the extra space. I am so used to starting a new sentence with double spacing that I do it habitually. I will try to be better, but I am sure that it will come up in other patches I submit for issues. Thanks for bearing with me.

Re 3.6 - I was not happy with the original construct. Hence, I turned it into a complete sentence.

Re 3.11 - This change may conflict with a change to the same line in one of the '(optional)' patches. I cannot find that patch quickly now, but included the suggested change in this patch as well.

Re 3.14 - Fixed up the description for the HIDDEN constant. TLC is needed on StreamWrapperInterface.php to clean up the other constants as well as full file standards review.

Re 3.15 - I am laughing. I was attempting to keep this issue strictly focused on 'e.g.' and avoid 'i.e.'. Fixed text as requested.

Re 3.18 - I was just focused originally of the 'e.g.' portion of the comment. In the future, I will also clean up any docblock issues that include a 'e.g.' string fix.

Attached is an interdiff and patch (with -U6 switch) that address the comments from #3.

jhodgdon’s picture

Status: Needs review » Fixed

This is great! I'm going to go ahead and commit it to 8.0 and 8.1. That will likely conflict with your "optional" patch, but hopefully not too much.

A couple of notes:

  1. +++ b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php
    @@ -89,15 +89,16 @@ public function getLabels($include_calculated_cache_contexts = FALSE) {
        * - a calculated cache context ID, followed by a double colon, followed by
    -   *   the parameter for the calculated cache context, e.g. 'bar:some_parameter'
    +   *   the parameter for the calculated cache context; for example,
    +   *   'bar:some_parameter'.
    

    Just as a note, and you didn't introduce this... The docs say it's a double-colon but the example only has one colon in bar:some_parameter... one of them must be wrong? Separate issue though.

    Hm. Looking at the rest of the file I think it's a single colon. So I fixed this in the commit.

  2. +++ b/core/lib/Drupal/Core/Image/ImageInterface.php
    @@ -228,16 +231,16 @@ public function desaturate();
        *   (optional) An hexadecimal integer specifying the background color to use
    -   *   for the uncovered area of the image after the rotation. E.g. 0x000000 for
    -   *   black, 0xff00ff for magenta, and 0xffffff for white. For images that
    -   *   support transparency, this will default to transparent. Otherwise it will
    -   *   be white.
    +   *   for the uncovered area of the image after the rotation; for example,
    +   *   0x000000 for black, 0xff00ff for magenta, and 0xffffff for white. When
    +   *   NULL (the default) is specified, for images that support transparency,
    +   *   this will default to transparent; otherwise, it will default to white.
    

    Much nicer thanks!

  • jhodgdon committed aaabb05 on 8.1.x
    Issue #2627018 by Lars Toomre: Some fixes for 'e.g.' in docblocks and...
Lars Toomre’s picture

Thank you @jhodgon. I will start working shortly on the next incremental patch addressing 'e.g.'. My notes from this one include addressing the content of a docblock if it is touched and not just the 'e.g.' sub-string. Attempt to keep the patch size using -U6 switch around 50k in size. Anything else I missed?

jhodgdon’s picture

No, I thought this round went well! Thanks again!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.