Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Issue tags: +Novice, +Needs backport to D7

Good idea. Probably a good Novice project -- just add links to the main topic, following the link syntax:
http://drupal.org/node/1354#links

bartlantz’s picture

Status: Active » Needs review
FileSize
6.33 KB

Here's a patch. I added a link back to the Field API document from all of the subtopics.

bartlantz’s picture

This is the back port of the patch to drupal 7.

bartlantz’s picture

Assigned: Unassigned » bartlantz

I've assigned the issue to myself.

jhodgdon’s picture

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

Thanks for the patches! We need to leave this issue as Drupal 8 until it's resolved for D8, and normally I recommend not porting a d8 patch to d7 until the d8 patch has been accepted and committed (saves time and effort, since often patches go through a couple of iterations before they are accepted).

Looking at the patch for D8 in #2, I see:

+ * See: @link field Field API data structures @endlink.

This should just be
@see field
which will make the desired link back to the field topic.

Also, in this issue what we wanted was for the @defgroup topic sections that are subtopics of the Field API topic to have this link, not the individual functions that are listed in the topics.

bartlantz’s picture

Status: Needs work » Needs review
FileSize
2.53 KB

Okay, I think I've got it. I've put a "@see field" link in all of the @defgroup subtopics that the main Field API links to. (Sorry about switching versions, I was thinking it switched it for just that patch.)

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Perfect, thanks! (Assuming the committers agree, after it is committed would be the time to port this to D7.)

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Makes sense, committed/pushed to 8.x., moving to 7.x to webchick.

bartlantz’s picture

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

Patch that puts a "@see field" link in all of the @defgroup subtopics that the main Field API links to. Here's the backport of the patch to Drupal 7.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome improvement!

Committed and pushed to 7.x. Thanks!

jhodgdon’s picture

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

This needs to be reopened. The @see links did not work. Apparently we need to make them @link instead of using @see.
http://api.drupal.org/api/drupal/modules--field--field.attach.inc/group/...

bartlantz’s picture

That's strange! Here's a new patch. I wasn't sure what to put as the link text, I used the following
See @link field Field API @endlink.

Let me know if should be something else. Thanks!

bartlantz’s picture

Status: Needs work » Needs review

oops, I forgot to change the status to needs review.

jhodgdon’s picture

The link text is fine... and this looks OK... but I'm wondering if we should say something like "See (link) for information about the other parts of the Field API", or something like that to explain why we are putting that See line there? Or maybe "See also (link)" rather than just "See (link)"? (@see normally formats to a See Also section).

Note also that this patch will interfere with the patch for #1373194: Field Attach API topic/group page should link to Field Language API page. One of them will need a reroll when the other is committed.

bartlantz’s picture

That sounds like a good idea to add more explanation. I used the longer text because I think it might look strange to have a 'See also ' that wasn't formatted with the See Also section. Also the line was over 80 characters so I folded it onto another line.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

That looks totally reasonable. Thanks!

webchick’s picture

Category: feature » task
Status: Reviewed & tested by the community » Fixed

Looks like a great change, thanks!

Committed and pushed to 8.x and 7.x. This looks mis-filed as a feature request.

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