Part of #2339219: [meta] Finalize URL generation API (naming, docs, deprecation).

Problem/Motivation

The change record says:

"\Drupal::url() *may* also become deprecated and *may* be removed. New code should use the Url class directly...

That was from the original change record draft. However, for the beta release, either it's deprecated, or it's not and we will keep it in D8.

Proposed resolution

  • Retain \Drupal::url() as a convenience wrapper for Url::toString().
  • Update the documentation to refer to Url.
  • Remove the paragraph from the change record.

Remaining tasks

At some point we may wish to change it to use Url internally instead so that we don't have so many different code paths.

User interface changes

N/A

API changes

None.

CommentFileSizeAuthor
#7 interdiff.txt1.93 KBeffulgentsia
#7 url-l-docs-2347831-7.patch9.27 KBeffulgentsia
#4 url-docs-2347831.patch8.23 KBxjm
deprecate-drupal-url.patch1.31 KBxjm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Reviewed & tested by the community

Looks very familiar

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: Deprecate \Drupal::url() » Fix documentation for \Drupal::url() to reference Url() (and fix the change record)
Assigned: Unassigned » xjm
Status: Reviewed & tested by the community » Needs work

Discussed with @larowlan, @tim.plunkett, and @alexpott. We decided to leave \Drupal::url() in as a convenience wrapper for new Url(..)->toString() (which means we are keeping it in 8.x.x) and just update the docs to reference Url.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: -beta blocker

Not a beta blocker then either, though I would encourage us to commit it before beta anyway as it will only be docs. Working on it now.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: Fix documentation for \Drupal::url() to reference Url() (and fix the change record) » Fix documentation for \Drupal::url(), \Drupal::l(), etc. (and fix the change record)
Issue summary: View changes
FileSize
url-docs-2347831.patch8.23 KB

The \Drupal::l() docs were also wrong now, so adding that to scope. This is what I have for now. Time to sleep.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented 
+++ b/core/lib/Drupal/Core/Url.php
@@ -88,10 +88,6 @@ class Url {
-   * In most cases, use Url::fromRoute() or Url::fromUri() rather than
-   * constructing Url objects directly in order to avoid ambiguity and make your
-   * code more self-documenting.
-   *

why remove this hunk?

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
FileSize
url-l-docs-2347831-7.patch9.27 KB
interdiff.txt1.93 KB

Reverted #6 and merged #2343661-23: Rename l() to _l() and url() to _url(), and document replacements.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Looks great! Thanks for clearing it up.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 64de978 and pushed to 8.0.x. Thanks!

  • alexpott committed 64de978 on 8.0.x 
    Issue #2347831 by xjm, effulgentsia: Fix documentation for \Drupal::url...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Assigned: xjm » Unassigned

#6 was because we went back and forth about it and didn't actually come to a consensus as to whether we should recommend using Url::fromRoute() over new Url() generally, but not a big deal either way. Thanks everyone!

Status: Fixed » Closed (fixed)

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

donquixote’s picture

donquixote CreditAttribution: donquixote commented

I don't understand why @param and @return docs were removed from \Drupal::url() and \Drupal::l().
These docs are meant at least 50% for the machine (the IDE, mostly), which can't do anything with the "See \Drupal\Core\Url::fromRoute() for detailed documentation."

The proposed changes in #2363523: Docblock / cleanup in \Drupal will re-introduce the @param and @return, albeit with only a short description, and the "for detailed documentation, see ..." still in place.

The other question is, for url(), why do we refer to \Drupal\Core\Url::fromRoute(), and not to \Drupal\Core\Routing\UrlGeneratorInterface::generateFromRoute(), which is the actual function being called?

More discussion over there, I would say.