Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Title: Inspect referrer and automatically fix version number for links » Use file name rather than full URL for links to Form API Reference page
Project: API » Drupal core
Version: 6.x-1.x-dev » 8.x-dev
Component: Code » documentation
Category: feature » task
Issue tags: +Needs backport to D6, +Novice, +Needs backport to D7

Actually, what we should be doing here is just using the bare file name for the link, instead of the full URL. That should turn into the proper link with the right version suffix. So, we have in includes/form.inc currently:

  * workflow, see the
  * @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html Form API reference @endlink
 

It should just say

 * workflow, see forms_api_reference.html

(without the @link stuff). I realize that this will not be quite as friendly, since it won't have the link title... so I guess it could say:

  * workflow, see the Form API reference (forms_api_reference.html)

(may need some line re-wrapping after that).

Should be a good Novice task. Should be backported to D7 and 6 as well. If there is any other existing full URL link in the code to the same URL in the code, that should be fixed too. And I also noticed that on the Drupal 6/7/8 landing pages on api.drupal.org, the link to Form API reference is broken there, so that needs to be fixed too (but that is in the Documentation repository, so let's get this other stuff fixed first).

And as a note to self: When a patch is submitted here, I should test it out on my API test site and verify that the links work.

nmudgal’s picture

Could not find any other link in file.
Patch is attached after removing @link & just putting filename as mentioned.

Thanks

nmudgal’s picture

Version: 8.x-dev » 7.x-dev
FileSize
731 bytes

For D7.

nmudgal’s picture

Status: Active » Needs review
jhodgdon’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs review » Needs work

Please leave this at 8.x until we get a good patch for 8.x committed. Then we can worry about the D7 port. Thanks!

So regarding the patch in #2, aside from the fact that the testbot can't apply it (not sure why):

+ * workflow, see the Form API reference (forms_api_reference.html)
  * and the

Probably this "and the" can be moved up to the previous line?

jhodgdon’s picture

Also, there are some other links. A quick grep found:

includes/ajax.inc: *   @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html/7 Form API Reference @endlink
includes/form.inc: * @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html Form API reference @endlink
includes/form.inc: *     @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html/7#tree #tree @endlink
modules/field/field.api.php: * @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html Form API @endlink
nmudgal’s picture

yes that's what I was wondering why bot is checking against version-7.
Update patch.

nmudgal’s picture

Status: Needs work » Needs review

review.

jhodgdon’s picture

Status: Needs review » Needs work

still needs to fix the other links in #6

nmudgal’s picture

Status: Needs work » Needs review
FileSize
3.48 KB

Updated as in #6, please review.

Thanks

jhodgdon’s picture

Status: Needs review » Needs work

Thanks!

This one needs some thought:

@@ -214,10 +212,9 @@ function drupal_get_form($form_id) {
  *     set.
  *   - values: An associative array of values submitted to the form. The
  *     validation functions and submit functions use this array for nearly all
- *     their decision making. (Note that
- *     @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html/7#tree #tree @endlink
- *     determines whether the values are a flat array or an array whose
- *     structure parallels the $form array.)
+ *     their decision making. (Note that the Form API reference
+ *     (forms_api_reference.html) determines whether the values are a flat
+ *     array or an array whose structure parallels the $form array.)

The original reads "Note that #tree determines whether...". With your patch, it would now read "Note that the Form API reference () determines ...". That doesn't make much sense. Probably it needs to say:

Note that #tree determines whether the values are... See the Form API reference (filename) for more information.

This one also needs some similar attention:

@@ -667,11 +667,10 @@ function hook_field_is_empty($item, $field) {
  * which widget to use. Widget types are defined by implementing
  * hook_field_widget_info().
  *
- * Widgets are
- * @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html Form API @endlink
- * elements with additional processing capabilities. Widget hooks are typically
- * called by the Field Attach API during the creation of the field form
- * structure with field_attach_form().
+ * Widgets are, see the Form API reference (forms_api_reference.html) elements
nmudgal’s picture

Updated according to #11.

Thanks.

nmudgal’s picture

Status: Needs work » Needs review
nmudgal’s picture

If that patch is good to go, I would be working on d7 one?

Thanks

jhodgdon’s picture

Status: Needs review » Postponed

On D8/7 and when to work on the next patch - please see http://drupal.org/node/1488414#comment-5748422

Meanwhile... This patch is much closer, thanks! A few little items:

a) In the ajax.inc part of the patch, the new sentence needs a . at the end.
b) The form.inc section looks good.
c) In the field.api.php section, the original said "Widgets are Form API elements...". Let's keep that text.

Actually... I just tested this in my test version of the api.drupal.org site. Sigh. These links will not work anyway unless we do something different in the API module. Back to the drawing board... Sorry to send everyone on a wild chase to fix this.

jhodgdon’s picture

API issue to support this:
#1489000: Need a way to make a reliable link to an HTML file
When that is resolved, we can come back here and fix the links. Until then, it's a bit pointless.

jhodgdon’s picture

Status: Postponed » Needs work

That issue is now resolved, and the new code is on api.drupal.org. So it's time to go back and fix this. See comments in #15 (a-c).

nmudgal’s picture

Status: Needs work » Needs review
FileSize
3.54 KB

Updated according to #15.

jhodgdon’s picture

So... I actually just fixed up the API module so that we can do any of the following:
.... filename.html ...
.... (filename.html) ...
.... @link filename.html Link text @endlink
That software fix should be deployed on api.drupal.org in a day or two (actually maybe later today).

Do you think that @link would be better than what we're doing now in this patch (which is using () around the filename)? That is, we can take

 @link http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.html/7 Form API Reference @endlink

and change it to

 @link forms_api_reference.html Form API Reference @endlink

Thoughts?

Robin Millette’s picture

What about using markdown link syntax? Something like

This is [an example](http://example.com/ "Title") inline link.

[This link](http://example.net/) has no title attribute.

jhodgdon’s picture

Robin: Sorry, that is not going to happen. We are using @ tags. This is just a small refinement of what we already have, and we're not going to completely change it at this point.

nmudgal’s picture

+1 for this as this documentation is always meant to be for devs & it looks more decent rather than putting (see for more information ... thing)

@link forms_api_reference.html Form API Reference @endlink
droplet’s picture

But where to find forms_api_reference.html ??

nmudgal’s picture

@droplet
It would link to http://api.drupal.org/api/drupal/developer%21topics%21forms_api_referenc... as discussed in #15, #16, #19

droplet’s picture

@nmudgal,

API module will do this job but your IDE won't. so How to find it quickly? Google?

droplet’s picture

if no links, it maybe better..

@link API SITE: forms_api_reference.html Form API Reference @endlink

nmudgal’s picture

As I said in #22, it is intended for devs & they already know where to find FAPI reference, so think it is sufficient. But #26 sounds ok to me too.

Anonymous’s picture

I would rather see @link forms_api_reference.html Form API Reference @endlink as it stands out better.

jhodgdon’s picture

#26 will not work with the API module.

Our choices are either #28 or the most recent patch.

jhodgdon’s picture

Status: Needs review » Needs work

I see several votes (mine too) for #28. Let's get a new patch.

Albert Volkman’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs work » Patch (to be ported)

This looks like it was resolved with this issue-

http://drupal.org/node/1552412

Moving this back to 7.x.

Cameron Tod’s picture

Status: Patch (to be ported) » Needs review
FileSize
4.11 KB

Here's a backport to D7. Note that I had to change the 'values' comment for drupal_get_form() to make sense in context - hope this is ok and not too much like scope creep.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, thanks! I'll get it committed shortly.

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x. Thanks!

Albert Volkman’s picture

Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)

No love for D6? :)

Cameron Tod’s picture

Assigned: Unassigned » Cameron Tod
Status: Patch (to be ported) » Needs review
FileSize
607 bytes

One love, always :D

jhodgdon’s picture

Status: Needs review » Needs work
- * @link http://api.drupal.org/api/file/developer/topics/forms_api_reference.html/6 reference @endlink
- * and the @link http://drupal.org/node/204270 Form API guide. @endlink
+ * @link forms_api_reference.html @endlink and the
+ * @link http://drupal.org/node/204270 Form API guide. @endlink

The first link is missing the link text (which in the original says "reference".

And is this really the only place in all of the D6 source where forms_api_reference.html appears?

Cameron Tod’s picture

Status: Needs review » Needs work

Ok. You're right too, I found another reference:

    $output .= '<li>'. t('register_globals is <strong>turned off</strong>. If you need to use forms, understand and use the functions in <a href="@formapi">the Drupal Form API</a>.', array('@formapi' => url('http://api.drupal.org/api/group/form_api/6'))) .'</li>';
jhodgdon’s picture

No, that is a reference to http://api.drupal.org/api/group/form_api/6 -- which is a different page.

Albert Volkman’s picture

[6.x] $ grep -rI 'forms_api_reference' .
./includes/form.inc: * @link http://api.drupal.org/api/file/developer/topics/forms_api_reference.html/6 reference @endlink

That's all I found.

jhodgdon’s picture

OK, good! Then let's get that one fixed correctly (see #37). :)

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
626 bytes

Done.

Cameron Tod’s picture

+++ b/includes/form.incundefined
@@ -41,8 +41,8 @@
- * @link http://api.drupal.org/api/file/developer/topics/forms_api_reference.html/6 reference @endlink
- * and the @link http://drupal.org/node/204270 Form API guide. @endlink
+ * @link forms_api_reference.html Form API Reference @endlink and the
+ * @link http://drupal.org/node/204270 Form API guide. @endlink

Should this be "reference" instead of "Form API Reference"? That's how it was previously.

Also, apologies for my sloppy comment above, I was interrupted on my way out the door and probably shouldn't have clicked "Save".

Cameron Tod’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks -- the patch in #43 looks good.

jhodgdon’s picture

Assigned: Cameron Tod » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks! At long last, committed to 6.x.

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D6, -Novice, -Needs backport to D7

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