Download & Extend
Page Title » Issues

Cleanup token integration

Posted by Dave Reid on October 15, 2010 at 8:45pm
Project:Page Title
Version:7.x-2.x-dev
Component:Code
Category:task
Priority:normal
Assigned:Dave Reid
Status:needs work
Issue tags:token

Issue Summary

Working on a patch and summary to help improve and simplify the token integration in Page Titles for D7 by re-adding the dependency on token module...

Login or register to post comments

Comments

#1

Posted by Dave Reid on October 15, 2010 at 11:30pm
Status:active» needs review

Patch summary:

  1. Re-adds the module dependency on token module in contrib. This is done to help re-use what's available in token module like...
  2. Token context validation on the page title fields. So with just a little code you get automatic errors if someone tries to use the [user:name] token on the Comment Reply pattern.
  3. Use of the 'token tree' UI so you don't have to do any custom token listing code.
  4. Moves the token implementation to page_title.tokens.inc. Note this is automatically supported by core and doesn't rely on Token module to just work.
  5. Move the page-title token underneath the [current-page] token namespace, which is provided by Token module and not by core. It also provides the [current-page:page-number] token so that's done for you.
  6. Use the proper $options['sanitize'] rather than providing both the page-title and page-title-raw tokens.

So sorry that it's not quite that easy to drop the dependency on Token module for D7, but we do want to help provide everyone with as much re-usable code that we can hopefully get into core for D8.

AttachmentSize
942978-pagetitle-token-cleanup-D7.patch 16.77 KB

#2

Posted by Dave Reid on October 15, 2010 at 11:36pm

Adding tag...

#3

Posted by nicholasThompson on November 1, 2010 at 11:21pm
Status:needs review» fixed

Commited to DRUPAL-7--2. Thank you so much for the patch. MUCH appreciated!

#4

Posted by Dave Reid on November 11, 2010 at 3:41am
Status:fixed» needs review

Some small follow-ups that somehow didn't make it into the original patch:
1. Support using vocabulary tokens on the forum container page title
2. Token replacement should request the unsanitized tokens since it applies filter_xss() later.

AttachmentSize
942978-pagetitle-token-cleanup-followup-D7.patch 3.98 KB

#5

Posted by nicholasThompson on November 11, 2010 at 9:21am

Is there a coding preference amongst the community for comma's at the end of in-line arrays? http://drupal.org/coding-standards#array doesn't seem to state either way (apart from the example not having one)
Eg

$some_array = array('hello', 'world', 'foo' => 'bar');

vs
$some_array = array('hello', 'world', 'foo' => 'bar',);

Sorry if it seems picky - but I just noticed a few lines of that patch which only seem to change the trailing comma.

#6

Posted by Dave Reid on November 11, 2010 at 9:28pm

If your array is on one line and less than 80 characters long, it's the code standard to not use a trailing comma. But if you have a long array with each element on a separate line, you're supposed to use a trailing comma. It's detailed exactly on http://drupal.org/coding-standards#array

#7

Posted by pillarsdotnet on February 9, 2011 at 9:34pm

Re-rolled with some additional fixes:

  • Wrap long comments at 80 columns.
  • Minor grammatical corrections within comments.
  • Wrap long conditional expressions at infix operators.
  • Add parentheses around assignment statements within conditional expressions.
  • Collapse nested conditional statements where possible.
  • Improve logic in function node_page_title_alter().
AttachmentSize
page_title.patch 14.09 KB

#8

Posted by pillarsdotnet on February 9, 2011 at 10:23pm
Title:Re-add token module dependency and cleanup token integration» Cleanup token integration

More of the same.

AttachmentSize
page_title.patch 14.94 KB

#9

Posted by nicholasThompson on February 9, 2011 at 10:36pm

Ah I just applied the patch from #7 and refreshed here to reply... What changed between #7 and #8?

#10

Posted by pillarsdotnet on February 10, 2011 at 1:47am

Interdiff is your friend.

AttachmentSize
interdiff.patch 801 bytes

#11

Posted by nicholasThompson on February 10, 2011 at 12:46pm

Oooo Interdiff! Handy!

#12

Posted by nicholasThompson on February 10, 2011 at 8:20pm
Status:needs review» fixed

Committed - thanks for the work - much appreciated!

#13

Posted by System Message on February 24, 2011 at 8:30pm
Status:fixed» closed (fixed)

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

#14

Posted by porg on September 29, 2011 at 3:42pm

Dear maintainer: Could you please add a short info concerning Drupal 7 and module dependency to the project page so that not every ordinary user must snoop through the issues queue, in order to get that basic info. Thanks! Something like:

Although the functionality of the contributed module token has moved into core in Drupal 7, page_title is still dependent on the module token to support some functions yet not included in core. (For details read: #942978: Cleanup token integration ).

#15

Posted by pillarsdotnet on September 29, 2011 at 4:18pm

@porg

You either need to re-open this issue (change its status to "needs work") or preferably, open a new issue and then add a comment here that links to it.

#16

Posted by porg on September 29, 2011 at 5:17pm
Status:closed (fixed)» needs work