Update remaining url() and _l() calls in comments/documentation

working on this issue

Working on this issue

thanks for trying this.

un-assigning since it has been a while.
If you have questions or partial work, please make a comment on the issue.

I got here from #2424525: Remove references to _l() and _url(). Man, what a nice number! It's a pity to see that closed. :)

I am uploading the patch from #2424525-1: Remove references to _l() and _url() here to get things started.

Note to reviewers:

+++ b/core/lib/Drupal/Core/Routing/UrlGeneratorInterface.php
@@ -66,10 +66,6 @@
-   *   - 'entity_type': The entity type of the object that called _url(). Only
-   *     set if _url() is invoked by Drupal\Core\Entity\Entity::uri().
-   *   - 'entity': The entity object (such as a node) for which the URL is being
-   *     generated. Only set if _url() is invoked by Drupal\Core\Entity\Entity::uri().

These doesn't seem to be used at all. I couldn't find the uri() methods on Entity class, and the similarly named url() method had no reference to this properties nor any other function call that might work up on this.

@hussainweb, do you think you caught all of them or are you still working on this? I'm not going to work on this issue directly so I can do proper reviews instead.

@mpdonadio: No, I had actually mentioned this in my other comment but forgot to copy it here. Basically, I am calling it a day and plan to catch the remaining tomorrow. I just wanted to get started on this.

Setting to Needs Work accordingly.

Should this be taken up during the sprints at DrupalCon Barcelona?

If so, then it should be tagged with Barcelona2015.

Yeah, this slipped through the cracks. Current patch doesn't apply either. Since _url() and _l() are long gone, usages need to be found with search and/or grep.

start working on reroll

Here's the reroll.

It's a random failure as the changes are only in comments. Retesting...

It's a random failure as the changes are only in comments. Retesting...

+++ b/core/lib/Drupal/Core/Render/Element/Link.php
@@ -38,17 +38,18 @@ public function getInfo() {
+    // By default, link options to pass to the link generator are normally set

Should this contain the full class name?

The provided patch is not only removing the calls but updating the documentation, so therefore I'm updating the title.

I mentored on this issue.

I think it's OK as it is. Thanks!

After applying the patch I still appear to have 22 occurences of _l() - for example in core/modules/system/src/Tests/Common/UrlTest.php
And 3 occurrences of _url() all in core/lib/Drupal/Core/Routing/UrlGeneratorInterface.php

Rerolled again>

And this fixing #28 report

Thanks! I checked and I think this patch covers all the _url() and _l() calls now... not 100% sure about _url() because I checked with grep, nad there are a lot of other functions called things like check_url() etc. that complicated things... but fairly sure. _l() is definitely covered.

So... The new patch looks mostly OK but there are a few problems:

1. +++ b/core/lib/Drupal/Core/Language/language.api.php
   @@ -159,7 +159,7 @@
   + * translated link text before going through the link generator, which will just handle the
   

   This documentation line goes over 80 characters, and needs to be rewrapped.

2. +++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php
   @@ -267,7 +267,7 @@ public function getLocalTasksForRoute($route_name) {
   +            // Normally, the link generator compares the href of every link with the current
   

   over 80 characters in code comment lines is also a standards violation. Needs rewrap.

3. +++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php
   @@ -275,7 +275,7 @@ public function getLocalTasksForRoute($route_name) {
   -              // This tab has visible children
   +              // This tab has visible children.
   

   This change is technically out of scope for the issue... but that's probably OK.

4. +++ b/core/lib/Drupal/Core/Menu/menu.api.php
   @@ -437,9 +439,10 @@ function hook_system_breadcrumb_alter(\Drupal\Core\Breadcrumb\Breadcrumb &$bread
   + *   "route link", 'route_name' will be set, otherwise 'path' will be set.
   

   maybe fix the punctuation here too? It should be:

   ... will be set; otherwise, 'path' ...

5. +++ b/core/lib/Drupal/Core/Routing/UrlGeneratorInterface.php
   @@ -66,10 +66,11 @@
   +   *   - 'entity_type': The entity type of the object that called the URL generator.
   +   *     Only set if the URL generator is invoked by Drupal\Core\Entity\Entity::uri().
   

   over 80 characters, rewrap.

   Also, class name should start with \

6. +++ b/core/lib/Drupal/Core/Routing/UrlGeneratorInterface.php
   @@ -66,10 +66,11 @@
   +   *     Drupal\Core\Entity\Entity::uri().
   

   Class name should start with \

7. +++ b/core/modules/system/src/Tests/Common/UrlTest.php
   @@ -102,7 +103,8 @@ function testLinkAttributes() {
   +    // Test the active class in links produced by \Drupal\Core\Utility\LinkGeneratorInterface::generate()
   

   over 80 chars, rewrap

8. +++ b/core/modules/system/src/Tests/Common/UrlTest.php
   @@ -122,23 +124,23 @@ function testLinkAttributes() {
   +    // Test adding a custom class in links produced by \Drupal\Core\Utility\LinkGeneratorInterface::generate() and #type 'link'.
   

   over 80 chars, rewrap

9. +++ b/core/modules/system/src/Tests/Theme/FunctionsTest.php
   @@ -172,7 +172,7 @@ function testItemList() {
   +    // Turn off the query for the \Drupal\Core\Utility\LinkGeneratorInterface::generate() method to compare the active
   

   over 80 chars, rewrap

10. +++ b/core/modules/system/tests/modules/common_test/src/Controller/CommonTestController.php
    @@ -19,7 +19,7 @@ class CommonTestController {
    +   * Using #type 'link' causes these links to be rendered with the link generator.
    

    over 80 chars, rewrap

11. +++ b/core/modules/views/src/Tests/ViewStorageTest.php
    @@ -185,7 +185,7 @@ protected function displayTests() {
    +    // Enable the system module so the link generator can work using url_alias table.
    

    over 80 chars, rewrap

I stubbed _url() and _l() back into common.inc and ran a usage report in PhpStorm w/ the patch applied and didn't get and results. Also searched for ' _l(' and ' _url(' and didn't see anything. I'm confident the patch caught everything, but I did not review the actual patch.

Most of what is being fixed here (if not all) is references in docs and comments, so I doubt PhpStorm would find it ?

Thanks for your revision and your time.

This is the patch with fixed on #31

Find Usages in PhpStorm will catch @see references, which is why I also ran text searches on those specific strings to double check the grep/ag checks.

Almost there! Two spots still need a quick fix:

1. +++ b/core/lib/Drupal/Core/Routing/UrlGeneratorInterface.php
   @@ -66,10 +66,12 @@
   +   *     Drupal\Core\Entity\Entity::uri().
   

   This class name still needs to start with \

2. +++ b/core/modules/system/src/Tests/Theme/FunctionsTest.php
   @@ -172,7 +172,7 @@ function testItemList() {
   +    // Turn off the query for the \Drupal\Core\Utility\LinkGeneratorInterface::generate() method to compare the active
   

   This is still over 80 characters and needs rewrapping.

Thanks! Looks good now.