Rename \Drupal\Core\Url::toRenderArray() to renderArrayProperties()

Looks good, though I'd make the $title param optional $title = '' perhaps? If it's missing maybe put in the link or generate the URL?

How about introducing a Link value object instead of muddying the waters in the Url class? That would be similar to the distinction we had previously with l() and url() (which made a lot of sense, imo).

@amateescu - this came out of my concern that the current function doesn't currently return a valid render array, and the link element is the only thing that makes sense in terms of the context where it's used.

I think renaming the method to toRenderArrayFragment() would be the non-controversial change at this point. This method could be used (potentially) for the autocompete form element, something like:

$form['stuff'] = array(
  ...
  '#autocomplete' => TRUE,
) + $url->toRenderArrayFragment();

That's why I'd prefer to not limit its functionality to a link element.

Hello, folks!
I've just renamed the method. I hope I understood the task correct.

Thanks

How about just toRenderfragment()?

Rerolled patch.

Encountered simple conflicts in the following files:
core/modules/user/src/Form/UserCancelForm.php
core/modules/node/src/Form/NodeDeleteForm.php
core/modules/entity_reference/src/Plugin/Field/FieldFormatter/EntityReferenceLabelFormatter.php
core/lib/Drupal/Core/Form/ConfirmFormHelper.php

No longer exists in head:
core/modules/comment/lib/Drupal/comment/Form/DeleteForm.php

@tim.plunkett I addressed your comments. It looks like we really need title='' in the signature because there are some existing usages that don't include $title. Also I tried to return ConfirmFormHelper to the original intent in #5.

Wow, core moves fast!

+++ b/core/lib/Drupal/Core/Url.php
@@ -371,18 +371,25 @@ public function toArray() {
-  public function toRenderArray() {
+  public function toRenderArrayFragment($title = '') {
     if ($this->isExternal()) {
       return array(
+        '#type' => 'link',
+        '#title' => $title,
         '#href' => $this->getPath(),
         '#options' => $this->getOptions(),
       );

I'm showing it to eff and neither of us understand why, now that this *is* returning a full render array with a #type => link, it makes sense to rename the thing to RenderArrayFragment. Seems like this is now returning a complete render array, no?

In addition, the word "Fragment" is overloaded. Not only because of HTML Page fragments, but also because of URL fragments, and this is on the Url class. :)

If we get a re-roll of this without the rename, this should be safe for anytime post-beta. But setting to needs review first because I don't quite parse peter's concerns in #7.

If, on the other hand, we decide to not have the Url class making assumptions about #type being link, then effulgentsia suggests maybe toRenderArrayProperties().

I'm fine with it being #type => link, but in that case I agree that we shouldn't change the method name. The whole motivation was since it didn't include a #type

change the method name back.

This satisfies the requirements of #32 and #34 and it is green.

1. +++ b/core/lib/Drupal/Core/Url.php
   @@ -379,18 +379,25 @@ public function toArray() {
   +   * @param string $title
   +   *   The title for the render array.
   +   *
   ...
   -  public function toRenderArray() {
   +  public function toRenderArray($title = '') {
   

   I don't think this is a sane idea, if you want to add it, just use += later or earlier. We are basically treating easy to use vs. sanity. Why should a url care about titles. If you want that, move this code to the link class

2. +++ b/core/lib/Drupal/Core/Url.php
   @@ -379,18 +379,25 @@ public function toArray() {
   +        '#type' => 'link',
   +        '#title' => $title,
   

   Same thing as before, a URL is not a link

Re: #38: Yup. Also #6, #9 and #10 above.

In the face of a lack of consensus - I sought a final decision from alexpott. This is the decision:

The method should be renamed to renderArrayProperties()

It will otherwise stay functionally the same and not provide a #type or #title.

Let's see how well phpstorm handles this.

simple method name fix - looks good

Rerolled.

@tim.plunkett - should we remove the method after that goes in instead?

I really kinda feel that this issue is pointless.

Let's do that in 9, its not worth to do that at all anymore in D8.

We could do it in a late minor release of 8.1.x with a deprecation, if we're really going to make the full change in 9.x.