Problems with documentation of options in Url class

Wow. That is truly stupid. It looks like the main/only way to set attributes on a link is that you have to put them on the URL object. ?!? There is apparently no way to set them on the Link class, even though the Url class itself doesn't use them in any way, and the attributes are definitely part of the link.

Yeah, we should definitely document this. Thanks for filing this issue and making a patch!

So I think maybe we should mention that the options are used on the anchor tag if you use the \Drupal\Core\Link object to make the link. Someone could conceivably use a Url object without using a Link object and they wouldn't get any attributes on their anchor tag. So, could you fix that?

Also, I checked all the methods on the Url object that deal with $options, and I noticed that on api.drupal.org, not all of them are making the links. Could you fix this, as long as we're fixing this class doc up? We need where it says:

see static::fromUri() for details.

for it instead to say

see \Drupal\Core\Url::fromUri() for details.

And one more thing: it seems like the $options variable and the get/set option/options methods on this class should also say "See \Drupal\Core\Url::fromUri() for details on the options."

And... I don't think we need to be documenting the $options in 3 places. Let's have them all say to see the fromUrl() method for details. Or else we should add the documentation to the other methods that currently say to see the fromUrl() method for details.

Adding this to the issue summary.

I hope this helps you.

Thanks for the new patch! It's pretty close. Just needs a small fix in a few places, plus a few substantive fixes... it doesn't look like my comments in #4 were taken into account?:

1. Where you're talking about attributes:
   

   +   *   - 'attributes': An associative array of HTML attributes to apply to the
   +   *     anchor tag.
   

   The thing is, the Url object doesn't have an anchor tag. So here we need to make it clear that these attributes will be added to the anchor tag if you use the \Drupal\Core\Link object to make the link. I suggested this in #4. Please take care of it (in all of the places it is mentioned). Thanks!

2. Also, let's just document options in ****one place*** and remove all of the other ones, replacing them with the line saying to see the \Drupal\Core\Url::fromUri() for details. I also asked for that in #4. Thanks!

3. +++ b/core/lib/Drupal/Core/Url.php
   @@ -56,6 +56,8 @@ class Url {
   +   * @see \Drupal\Core\Url::fromUri() for details on the options.
   

   An @see tag cannot have extra text, just the name of a function, class, method, etc. See https://www.drupal.org/node/1354#see for details.

   So this should just use the word "See" not @see. And it needs to be moved up above the @var.

4. +++ b/core/lib/Drupal/Core/Url.php
   @@ -638,6 +646,8 @@ public function setRouteParameter($key, $value) {
   +   * @see \Drupal\Core\Url::fromUri() for details on the options.
   

   See above note about @see vs. See. Needs to be above the @return here.

5. +++ b/core/lib/Drupal/Core/Url.php
   @@ -651,6 +661,8 @@ public function getOptions() {
   +   * @see \Drupal\Core\Url::fromUri() for details on the options.
   

   See above.

6. +++ b/core/lib/Drupal/Core/Url.php
   @@ -667,6 +679,8 @@ public function getOption($name) {
   +   * @see \Drupal\Core\Url::fromUri() for details on the options.
   

   See above.

7. +++ b/core/lib/Drupal/Core/Url.php
   @@ -682,6 +696,8 @@ public function setOptions($options) {
   +   * @see \Drupal\Core\Url::fromUri() for details on the options.
   

   See above.

Oh, sorry.
Before i send a new patch i need to ask.
Is this right?
You said above the @return, but i don't know if above the @param too.

/**
 * Returns the URL options.
 *
 * See \Drupal\Core\Url::fromUri() for details on the options.
 *
 * @return array
 */

/**
 * Gets a specific option.
 *
 * See \Drupal\Core\Url::fromUri() for details on the options.
 *
 * @param string $name
 *   The name of the option.
 *
 * @return mixed
 *   The value for a specific option, or NULL if it does not exist.
 */

/**
 * Sets the URL options.
 *
 * See \Drupal\Core\Url::fromUri() for details on the options.
 *
 * @param array $options
 *   The array of options.
 *
 * @return $this
 */

/**
 * Sets a specific option.
 *
 * See \Drupal\Core\Url::fromUri() for details on the options.
 *
 * @param string $name
 *   The name of the option.
 * @param mixed $value
 *   The option value.
 *
 * @return $this
 */

Yes, above @param and @return is correct.

However, for functions that have a $options parameter, the note should be with that parameter docs. So something like this:

* @param array $options
*    An array of options. See  \Drupal\Core\Url::fromUri() for details on what it contains.

Similarly, if the return value is the options, we should have something like this:

* @return array
*  The array of options. See  \Drupal\Core\Url::fromUri() for details on what it contains.

Thanks!

Note: I didn't check the word wrapping in #10. Documentation lines should be less than 80 characters long.

Thank you for your answers.
Sorry if i stole a little bit of your time.

Here's the new patch.

Not at all! Reviewing patches for API documentation in Drupal is what I do. A lot. I don't mind.

Anyway, this is looking better! A few small suggestions, and I think we'll be done in one more patch:

1. +++ b/core/lib/Drupal/Core/Url.php
   @@ -101,21 +103,7 @@ class Url {
   +   *   See the \Drupal\Core\Url::fromUri() for details.
   

   Let's take out the word "the" here. And in the other methods that have the same wording.

2. +++ b/core/lib/Drupal/Core/Url.php
   @@ -265,6 +239,9 @@ public static function fromUserInput($user_input, $options = []) {
   +   *     added to the anchor tag if you use the \Drupal\Core\Link object to make
   

   Hah. I think you took my exact wording. And when I look at it now, I think maybe instead of "object" near the end of this line, it should say "class"?

3. +++ b/core/lib/Drupal/Core/Url.php
   @@ -637,7 +614,11 @@ public function setRouteParameter($key, $value) {
   +   * See \Drupal\Core\Url::fromUri() for details on the options.
   +   *
       * @return array
   +   *   The array of options. See \Drupal\Core\Url::fromUri() for details on what
   +   *   it contains.
   

   I think we don't need both of these additional lines. Let's take out the top one.

4. +++ b/core/lib/Drupal/Core/Url.php
   @@ -663,8 +646,11 @@ public function getOption($name) {
   +   * See \Drupal\Core\Url::fromUri() for details on the options.
   +   *
       * @param array $options
   -   *   The array of options.
   +   *   The array of options. See \Drupal\Core\Url::fromUri() for details on what
   +   *   it contains.
   

   Same here, don't need both additional lines. Let's get rid of the top one.

New patch provided fixing suggestions from comment #13

+++ b/core/lib/Drupal/Core/Url.php
@@ -636,8 +613,11 @@ public function setRouteParameter($key, $value) {
+   * See \Drupal\Core\Url::fromUri() for details on the options.

This line still needs to be removed (the information is there a few lines down in the @param or @return).

So... the only other thing that kind of jarred me was that the wording about "see..." was a bit different in some places than others. But it is very clear everywhere, so I guess I won't worry about it. Let's just delete that one line and we'll be done. Thanks!

Just incorporated the above said change.

Oh sorry, I guess there were two places that needed the same fix. Here's the other one:

+++ b/core/lib/Drupal/Core/Url.php
@@ -662,9 +643,11 @@ public function getOption($name) {
    * Sets the URL options.
+   * See \Drupal\Core\Url::fromUri() for details on the options.
    *
    * @param array $options
-   *   The array of options.
+   *   The array of options. See \Drupal\Core\Url::fromUri() for details on what
+   *   it contains.

In this documentation block, we also do not need both of the lines saying "see whatever for details".

The top one needs to go away.

The one in @param needs to stay. Thanks!

Updated patch with suggested changes.

+++ b/core/lib/Drupal/Core/Url.php
@@ -647,9 +647,6 @@ public function getOption($name) {
   /**
-   * Sets the URL options.
-   * See \Drupal\Core\Url::fromUri() for details on the options.
-   *
    * @param array $options
    *   The array of options. See \Drupal\Core\Url::fromUri() for details on what
    *   it contains.

@jhodgdon Maybe we need keep "Sets the URL options" ??

Yes, we need "Sets the URL options". This tells us what the function does, and all functions need a one-line description. We just don't want two copies of "See .... for details". See #17. And #15.

This fix the comment mentioned above on #20.

@jhodgdon Looking a little in Url class I think that we could do a slightly better job of wrapping.
For example, the comments in the "fromRoute" function.
This would be within the scope of this task?

RE #23, I don't think that is within the scope of this issue. I would only say to rewrap lines that are related to the options (which is what this issue is about).

In which case, this patch looks fine. Thanks!

Tagging this issue.

Committed 5be833e and pushed to 8.1.x and 8.2.x. Thanks!