[drupal-docs] [task] Broken links on handbook's module pages

Fri Oct 7 03:48:29 UTC 2005

Issue status update for 
http://drupal.org/node/32078
Post a follow up: 
http://drupal.org/project/comments/add/32078

 Project:      Documentation
 Version:      <none>
 Component:    User Guide
 Category:     tasks
 Priority:     normal
 Assigned to:  webchick
 Reported by:  webchick
 Updated by:   webchick
 Status:       active
 Attachment:   http://drupal.org/files/issues/makeadminhelp_3.php (7.29 KB)

Ok, I've submitted a new patch here:
http://drupal.org/node/26139#comment-48325

I've attached the current script, although it still has some bugs in
it:

1. It only does the t('...
') thing if the  contains the string "You can".. silly
short-sightedness of me.. I need to change it to check if the next line
contains
 and if so, go through the same steps I go through for a "youcan"
block.
2. It only parses out  tags to go outside the t() statements (e.g.
$output .= ''. t(...) .'

'), but there are others (
 in block, for example) to which this rule should apply.
3. A couple modules randomly get an initial empty replacement value (so
replacement array looks like array(, '%something' => ). Need to figure
out what's causing that.
4. The Legacy module gets completely messed up. The last line from the
help module's help gets included here and then the next two lines all
run together, followed by a series of empty replacements (array(, , , ,
, )) and finally, $output restarts at the "Legacy has no configurable
options" line. Um. Yikes. :)

However, I ran out of time to troubleshoot these issues so I did some
of the final patch formatting manually. I will try and fix the script
this weekend (and also clean it up) so that it generates the expected
output.

webchick

Previous comments:
------------------------------------------------------------------------

Sat, 24 Sep 2005 04:20:37 +0000 : webchick

As was pointed out on the documentation mailing list, the "action links"
on each module handbook page are all relative links (for example
"admin/settings") and thus lead to 403 errors (if the user viewing them
doesn't have proper permissions) or 404 errors (if the module given is
not installed on Drupal.org).

The relative links are required because core/contrib admin help
documentation is generated from these handbook pages (so eventually,
http://www.drupal.org/ gets replaced by http://www.example.com/).
However, in the meantime, all of these handbook pages look like they
have errors in them. See the User module handbook page [1] for an
example. Clicking on any of the "You can:" links results in a 403 error
(unless of course you're Dries ;)).

My suggestion to solve this issue was to "tag" the links in the
handbook, using a naming convention something like:

*<strong id="drupal-link_admin-settings">administer >>
settings</strong>*

This would result in these links showing up as bold items, indicating
the menu paths a user should use, but NOT active hyperlinks that, when
clicked, would generate errors. They would also be uniquely tagged with
an ID which would be invisible to the browser, but which could be used
to regenerate the links later.

Upon export to admin help documentation, these pages could be run
through a script which would parse them with some regex magic, more or
less performing the steps:

1. Find an occurrence of the string: *(linkdescription)*
2. In linkpath, replace the '-'s with '/'s to build the relative link
path.
3. Replace the whole ... string with *<a
href="(linkpath)">(linkdescription)</a>*
4. Repeat for entire page.

What do you think? 

[1] http://drupal.org/handbook/modules/user

------------------------------------------------------------------------

Sat, 24 Sep 2005 18:20:36 +0000 : webchick

Attachment: http://drupal.org/files/issues/docparse.php (3.33 KB)

Here is a VERY rough script just to show as proof of concept.

to use:

1. Make a folder called "docparse" or whatever on your web server and
place this script inside it
2. Make the folder writable
3. Save http://drupal.org/book/export/docbook/279 as modules.xml in the
same folder as this script

What it does is first parse the contents of modules.xml (direct dump
from handbook) and creates modules_updated.xml. This simulates a dump
from the handbook after all of the links have been encoded in 
tags.

Then, it reverses the process and replaces the encoded links
with anchors and saves this as modules_replaced.xml. This simulates
taking the dump from the handbook and formatting it for admin/help use.

I haven't tested this overly thoroughly, but hopefully it will give you
an idea of the logic, etc.

------------------------------------------------------------------------

Sat, 24 Sep 2005 18:54:37 +0000 : Amazon

Here is an example of a Drupal core module patched with administration
help
http://svn.civicspacelabs.org/trunk/modules/aggregator.module
Here is an example of the desired output:

case 'admin/help#aggregator':
      return t('The news aggregator is a powerful on-site RSS
syndicator/news reader that can gather fresh content from news sites
and weblogs around the web.

Users can view the latest news chronologically in the main news
aggregator display [2] or by source [3]. Administrators can add, edit
and delete feeds and choose how often to check for newly updated news
for each individual feed. Administrators can also tag individual feeds
with categories, offering selective grouping of some feeds into
separate displays. Listings of the latest news for individual sources
or categorized sources can be enabled as blocks for display in the
sidebar through the block administration page [4]. The news aggregator
requires cron to check for the latest news from the sites to which you
have subscribed. Drupal also provides a machine-readable OPML file [5]
of all of your subscribed feeds.

You can

* administer your list of news feeds administer >>  aggregator [6].
* add a new feed administer >> aggregator >> add feed [7].
* add a new category administer >> aggregator >> add category   [8].
* configure global settings for the news aggregator administer >>
settings >> aggregator [9].
* control access to the aggregator module through access permissions
 administer >> access control >> permissions [10].

For more information, read the configuration and customization handbook
 aggregator page.

', array('%aggregator' => url('aggregator'), '%aggregator-sources' =>
url('aggregator/sources'), '%admin-block' =>url('admin/block'),
'%aggregator-opml' =>url('aggregator/opml'), '%admin-aggregator' =>
url('admin/aggregator'), '%admin-aggregator-add-feed' =>
url('admin/aggregator/add/feed'), '%admin-aggregator-add-category' =>
url('admin/aggregator/add/category'), '%admin-settings-aggregator' =>
url('admin/settings/aggregator'), '%admin-access' =>
url('admin/access')));

All relative links have been replace with variables and then added to
an array as pairs.  I am reviewing the output of your code right now.

Cheers,
Kieran

[2] http://drupal.org/%aggregator
[3] http://drupal.org/%aggregator-sources
[4] http://drupal.org/%admin-block
[5] http://drupal.org/%aggregator-opml
[6] http://drupal.org/%admin-aggregator
[7] http://drupal.org/%admin-aggregator-add-feed
[8] http://drupal.org/%admin-aggregator-add-category
[9] http://drupal.org/%admin-settings-aggregator
[10] http://drupal.org/%admin-access

------------------------------------------------------------------------

Sun, 25 Sep 2005 06:08:00 +0000 : webchick

Attachment: http://drupal.org/files/issues/makeadminhelp.php (3.98 KB)

Ok, here is another rough script that will output an expected *case
'admin/help#path':* for each core module. It also wraps each line in
its own t() function, per http://drupal.org/node/26139. To try it:

1. Put this script in a writable folder on your web server
2. Save http://drupal.org/book/export/docbook/279 as modules.xml in the
same folder as this script
3. The output is saved to a file called adminhelp.txt.

It's not quite "there" yet as there are still a couple bugs... For
instance, profile doesn't get picked up at all (you'll see a warning to
this effect when you load this), and there's at least one wrapping link
(on the taxonomy module page) which it doesn't pick up either. But
otherwise, this is working probably about 90%, and I'll try and sort
these other problems out tomorrow if I get a chance.

------------------------------------------------------------------------

Sun, 25 Sep 2005 18:11:53 +0000 : webchick

Attachment: http://drupal.org/files/issues/adminhelp.txt (47.78 KB)

Just as a helper.. this is the output of the above script, so you don't
have to go through all the setup routines to review it. :)

------------------------------------------------------------------------

Sun, 25 Sep 2005 18:22:36 +0000 : Amazon

This is one of those simple things that will make a huge difference.

The Docs team now has the ability to update the source documentation
here: http://drupal.org/handbooks/modules and with this script we can
just hand a file to the development team to roll a patch.

A big thanks to webchick for cranking this out.

Kieran

------------------------------------------------------------------------

Tue, 27 Sep 2005 09:08:05 +0000 : webchick

Attachment: http://drupal.org/files/issues/makeadminhelp_0.php (5.52 KB)

Ok, probably about 99% there now, I hope. :) Here is the updated script
(getting REALLY ugly now *lol*). Will post the updated output in a
second.

After talking to killes on IRC, he brought up the point that t() blocks
should be like:

'<p>'. t('...') .'</p>';

and for lists:

'<ul>' .t('<li>....</li>');
t('<li>...</li>') .'</ul>');

So the current script does that.

I still have two outstanding issues that I'm aware of (possibly more,
it's 5am :P):

1. When there are multiple links on one line (see: BlogAPI), it only
seems to find the first link/description and repeats it for all other
links on that line.

2. I cannot figure out how to generate a "normal" link with a Drupal
function call! l() and url() both appear to do basically the same
thing--format an internal link. However, Drupal module for example has
a link that is just cron.php... Which of course is not the same thing
as ?q=cron.php. Similarly, all external links (BlogAPI) get appended to
the querystring (?q=http://www.flickr.com/).

Any ideas on these (particularly the last one... the first one is
likely a fubar'ed regex)?

------------------------------------------------------------------------

Tue, 27 Sep 2005 09:08:40 +0000 : webchick

Attachment: http://drupal.org/files/issues/adminhelp_0.txt (54.09 KB)

And here is the output.

------------------------------------------------------------------------

Tue, 27 Sep 2005 10:56:49 +0000 : stefan nagtegaal

Why do you do:

<?php
    case 'admin/help#aggregator':
      $helptext = '';
      $helptext .= '<p>'. t('The news aggregator is a powerful on-site
RSS syndicator/news reader that can gather fresh content from news
sites and weblogs around the web.') .'</p>';
?>

You can also use:

<?php
    case 'admin/help#aggregator':
       $helptext  = '<p>'. t('The news aggregator is a powerful on-site
RSS syndicator/news reader that can gather fresh content from news sites
and weblogs around the web.') .'</p>';
       $helptext .= 'resume';
?>

And:

<?php
      return($helptext);
?>

Should be

<?php
      return $helptext;
?>

Furthermore I do really, really like this patch!

------------------------------------------------------------------------

Tue, 27 Sep 2005 11:00:14 +0000 : stefan nagtegaal

I also envourage you to use url() instead of l() and don't forget to t()
everything which should be translatable, also in links!

------------------------------------------------------------------------

Tue, 27 Sep 2005 19:55:55 +0000 : webchick

Attachment: http://drupal.org/files/issues/makeadminhelp_1.php (5.85 KB)

By JOVE I think I've got it!!

Thank you very much for your feedback, stefan! I implemented the
changes you suggested... apart from using url() in place of l(),
because I need to write the link's text as well as destination, which
url() doesn't let me do. :(

Attached is the horrendous script that makes it all possible. Will post
output in a second, and a patch against the current HEAD to this issue:
http://drupal.org/node/26139

Wish me luck! ;)

------------------------------------------------------------------------

Tue, 27 Sep 2005 19:56:29 +0000 : webchick

Attachment: http://drupal.org/files/issues/adminhelp_1.txt (54.27 KB)

And here is the output.

------------------------------------------------------------------------

Tue, 27 Sep 2005 20:49:13 +0000 : stefan nagtegaal

Why do you do:

<?php
      $helptext .= t('<li>control access to the aggregator module
through access permissions %admin-access.</li>', array('%admin-access'
=> l(t('administer >> access control >> permissions'),
'admin/access')));
?>

You should do:

<?php
      $helptext .= t('<li>control access to the aggregator module
through access permissions %admin-access.</li>', array('%admin-access'
=> l(t('administer'). ' &raquo; ' .t('access control') .' &raquo; '.
t('permissions'), 'admin/access')));
?>

This way, you are sure people could not translate 'administer >> access
control >> permissions' into other strings as 'administer', 'access
control' and 'permissions'...

---

Secondly I would do

<?php
    $help  = 'bla';
    $help .= 'bla, bla';
?>

instead of using $helptext.. It is 4 characters less each line, which
end up in quite a lot of bytes.. Results in a little faster page
building, though it's nitpicking...

This would improve your patch even more IMO...

------------------------------------------------------------------------

Wed, 28 Sep 2005 07:15:55 +0000 : Amazon

You'll need to take the following statement to each module as well:

For more information, read the configuration and customization handbook
 modulename page [11].

It's important that we drive traffic back to Drupal.org to improve the
documentation and allow for comments to be posted.  There's also much
more documentation there as well.

Kieran
[11] http://drupal.org/handbook/modules/modulename

------------------------------------------------------------------------

Wed, 28 Sep 2005 10:27:51 +0000 : stefan nagtegaal

I don't know if that's a good idea..
I don't want my site to lean on drupal.org for documentation, or
whatever..
If you truly want todo such a thing I would rather see a new module or
a settings which could be switched of..

------------------------------------------------------------------------

Wed, 28 Sep 2005 11:00:03 +0000 : Kobus

Stefan,

I don't think that the text Kieran talks about will be visible on your
site. As I understand it, it would only be in the documentation
released with the module? If indeed this would be on the page itself, I
also agree that this is a no-no. But it wouldn't harm to put that link
in the module's install.txt or readme.txt file.

Kobus

------------------------------------------------------------------------

Wed, 28 Sep 2005 14:02:49 +0000 : cel4145

Yes. It will be visible on every admin/help page. This was part of the
original documentation construction specs that Kieran posted this
summer. We could certainly discuss this, but raise a separate issue or
disucss in on the docs list.

------------------------------------------------------------------------

Wed, 05 Oct 2005 23:06:26 +0000 : webchick

Attachment: http://drupal.org/files/issues/makeadminhelp_2.php (6.05 KB)

Ok. Just a small update:

1. Changed $helptext to $output, per Dries (sorry, not quite as short
as $help but... :))
2. Added line at the bottom which links to Drupal.org module page (note
that this ONLY displays in the admin>help area of each module and not
anywhere else on the site. No one seemed to raise any horrible cons to
doing this on the docs list, so I assume this is ok anyway)

Outstanding issues:
1. Need to format the "You can" blocks, per killes's example here:
http://drupal.org/files/issues/agg.help
2. Need to get the >> out of the t() blocks, per stefan
3. There is some bug which will randomly split the string in the middle
for no reason, which is causing havoc with the translation stuff (e.g.
t('Fin') on one line, and t('ally...') on the other rather than
t('Finally...') on one line)
4. I'd like to figure out a way to store the handbook's version of the
module name for generating that link, so it comes out as CiviCRM, for
example, rather than civicrm.

------------------------------------------------------------------------

Thu, 06 Oct 2005 08:49:44 +0000 : stefan nagtegaal

Webchick, I think you can fix the outstanding issue 2 by doing this:

<?php
$output .= t('<li>control access to the aggregator module through
access permissions %admin-access.</li>', array('%admin-access' =>
l(implode(' &raquo; ', array(t('administer'), t('access control'),
t('permissions'))), 'admin/access')));
?>

This part of the code will make it a breadcrumb, which makes all the
parts separatly translatable and puts ' » ' between each part.

<?php
  implode(' &raquo; ', array(t('administer'), t('access control'),
t('permissions'))
?>

The rest should be a piece of cake...

Good luck, hope I helped you a little further... :-)

------------------------------------------------------------------------

Thu, 06 Oct 2005 09:46:08 +0000 : stefan nagtegaal

Attachment: http://drupal.org/files/issues/screenshot-aggregator-help.png (45.93 KB)

Another thing which would be nice, is if you wrap the list items through
theme('item_list').. See the example code underneath, which works....
(Tested on my own machine..)

So, instead of:

<?php
case 'admin/help#aggregator':
  $helptext = '<p>'. t('The news aggregator is a powerful on-site RSS
syndicator/news reader that can gather fresh content from news sites
and weblogs around the web.') .'</p>';
  $helptext .= '<p>'. t('Users can view the latest news chronologically
in the %aggregator or by %aggregator-sources. Administrators can add,
edit and delete feeds and choose how often to check for newly updated
news for each individual feed. Administrators can also tag individual
feeds with categories, offering selective grouping of some feeds into
separate displays. Listings of the latest news for individual sources
or categorized sources can be enabled as blocks for display in the
sidebar through the %admin-block. The news aggregator requires cron to
check for the latest news from the sites to which you have subscribed.
Drupal also provides a %aggregator-opml of all of your subscribed
feeds.', array('%aggregator' => l(t('main news aggregator display'),
'aggregator'), '%aggregator-sources' => l(t('source'),
'aggregator/sources'), '%admin-block' => l(t('block administration
page'), 'admin/block'), '%aggregator-opml' => l(t('machine-readable
OPML file'), 'aggregator/opml'))) .'</p>';
  $helptext .= '<p>'. t('You can') .'</p>';
  $helptext .= '<ul>'. t('<li>administer your list of news feeds
%admin-aggregator.</li>', array('%admin-aggregator' => l(t('administer
>>  aggregator'), 'admin/aggregator')));
  $helptext .= t('<li>add a new feed %admin-aggregator-add-feed.</li>',
array('%admin-aggregator-add-feed' => l(t('administer >> aggregator >>
add feed'), 'admin/aggregator/add/feed')));
  $helptext .= t('<li>add a new category
%admin-aggregator-add-category.</li>',
array('%admin-aggregator-add-category' => l(t('administer >> aggregator
>> add category  '), 'admin/aggregator/add/category')));
  $helptext .= t('<li>configure global settings for the news aggregator
%admin-settings-aggregator.</li>', array('%admin-settings-aggregator' =>
l(t('administer >> settings >> aggregator'),
'admin/settings/aggregator')));
  $helptext .= t('<li>control access to the aggregator module through
access permissions %admin-access.</li>', array('%admin-access' =>
l(t('administer >> access control >> permissions'), 'admin/access')));
  $helptext .= t('<li>set permissions to access new feeds for user
roles such as anonymous users at %admin-access.</li>',
array('%admin-access' => l(t('administer >> access control'),
'admin/access')));
  $helptext .= t('<li>view the %aggregator.</li>', array('%aggregator'
=> l(t('aggregator page'), 'aggregator'))) .'</ul>';
  return $helptext;
?>

Do this:

<?php
  case 'admin/help#aggregator':
    $output  = '<p>'. t('The news aggregator is a powerful on-site RSS
syndicator/news reader that can gather fresh content from news sites
and weblogs around the web.') .'</p>';
    $output .= '<p>'. t('Users can view the latest news chronologically
in the %aggregator or by %aggregator-sources. Administrators can add,
edit and delete feeds and choose how often to check for newly updated
news for each individual feed. Administrators can also tag individual
feeds with categories, offering selective grouping of some feeds into
separate displays. Listings of the latest news for individual sources
or categorized sources can be enabled as blocks for display in the
sidebar through the %admin-block. The news aggregator requires cron to
check for the latest news from the sites to which you have subscribed.
Drupal also provides a %aggregator-opml of all of your subscribed
feeds.', array('%aggregator' => l(t('main news aggregator display'),
'aggregator'), '%aggregator-sources' => l(t('source'),
'aggregator/sources'), '%admin-block' => l(t('block administration
page'), 'admin/block'), '%aggregator-opml' => l(t('machine-readable
OPML file'), 'aggregator/opml'))) .'</p>';
    $output .= '<p>'. t('You can') .'</p>';
    $items = array(
      t('administer your list of news feeds %admin-aggregator.',
array('%admin-aggregator' => l(implode(' » ', array(t('administer'),
t('aggregator'))), 'admin/aggregator'))),
      t('add a new feed %admin-aggregator-add-feed.',
array('%admin-aggregator-add-feed' => l(implode(' » ',
array(t('administer'), t('aggregator'), t('add feed'))),
'admin/aggregator/add/feed'))),
      t('add a new category %admin-aggregator-add-category.',
array('%admin-aggregator-add-category' => l(implode(' » ',
array(t('administer'), t('aggregator'), t('add category'))),
'admin/aggregator/add/category'))),
      t('configure global settings for the news aggregator
%admin-settings-aggregator.', array('%admin-settings-aggregator' =>
l(implode(' » ', array(t('administer'), t('settings'),
t('aggregator'))), 'admin/settings/aggregator'))),
      t('control access to the aggregator module through access
permissions %admin-access.', array('%admin-access' => l(implode(' » ',
array(t('administer'), t('access control'), t('permissions'))),
'admin/access'))),
      t('set permissions to access new feeds for user roles such as
anonymous users at %admin-access.', array('%admin-access' =>
l(implode(' » ', array(t('administer'), t('access control'))),
'admin/access'))),
      t('view the %aggregator.', array('%aggregator' => l(t('aggregator
page'), 'aggregator')))
    );
    $output .= theme('item_list', $items);
    return $output;
?>

As you can see I also implemented my point suggested in the comment
before...
The advantage of this is that we include even less HTML in the
helptext, which is goooood..

I attached a screenshot, to show you guys how it looks in my own
theme...

I think we are _really_ getting somewhere now!

------------------------------------------------------------------------

Thu, 06 Oct 2005 15:56:43 +0000 : chx

Less HTML is always good. Think of moving implode(htmltag, array()) to a
theme function. theme links , I think will be a good candidate.

------------------------------------------------------------------------

Thu, 06 Oct 2005 16:02:42 +0000 : killes at www.drop.org

Steef, chx:
If you had looked at the example that I gave you would have noticed
that individual li-s won't be strings on their own. They would lose too
much context.

------------------------------------------------------------------------

Thu, 06 Oct 2005 16:12:45 +0000 : Kobus

I agree with Gerhard here. Preservation of context is very important. I
have just started the translation of Drupal CVS, and it is hell to make
sure it is accurate. There are even bugs in my Drupal 4.6.x
installation's translation because of this same issue.

Regards,

Kobus

------------------------------------------------------------------------

Thu, 06 Oct 2005 18:44:41 +0000 : webchick

Re: steef's comments about doing:

<?php
implode(' &raquo; ', array(t('administer'), t('access control'),
t('permissions'))
?>

instead of:

<?php
t('administer >> access control >> permissions)
?>

I remember why I did that now.

l() escapes HTML entities in the title. So:

&raquot;

becomes:
&amp;raquot;

And thus instead of printing:

administer >> access control >> permissions

Any ideas on how to solve this? I tried setting the HTML attribute of
l() to true, but that doesn't seem to make a difference. :\

it prints:

administer &raquot; access control &raquot; permissions

------------------------------------------------------------------------

Thu, 06 Oct 2005 18:56:30 +0000 : webchick

Bah. I am stupid. Please ignore. :P

If anyone else is wondering though, the answer is to implode on >>
itself, not the HTML entities version of it.

Btw, » gave me ? in Firefox, so I changed it to >> as the handbook has
it.

Will post an update soon as I get killes' text working.