I'm making heavy use of service_links and love it, thanks!

It'd be really helpful to invoke an alter hook after collecting all the settings (especially node tags/substitution tokens) for a given node. In my case, my site has its own custom URL shortener, so I want to swap out the tokens related to URL shortening. However, you could also use this alter hook to do cool things like show/hide specific services on specific nodes or types of nodes, etc. I'm using this in 6.x-2.x, but I'm attaching patches for both that and 7.x-2.x (which is nearly identical except it adds an entry for hook_hook_info(), too).

Thanks!
-Derek

Comments

Patch still applies cleanly to 7.x-2.x-dev.

Simon -- did you test it? Did you review the code? Want to set this to RTBC? ;)

Thanks,
-Derek

No, I didn't (:-(), I was just triaging all the issues, and thought I'd bump this one to bring it to TheCrow's attention.

Oh well. ;) Thanks for verifying it still applies. Hopefully TheCrow will get a chance to review and commit.

Cheers,
-Derek

what against put the drupal_alter() before the filling of $settings['tag'] and $settings['show'] instead of the end?

<?php
 
switch ($settings['short_links_use']) {
    case
SERVICE_LINKS_SHORT_URL_USE_NEVER:
    ...
  }
 
// if needed all the variables could take a place under $settings['data']
  // to be changed later by this alter function.
 
drupal_alter('service_links_node_settings', $settings, $node);
 
$settings['tag'] = array(
    ...
  );
 
$settings['show'] = array(
    ...
  );
?>

Sorry, I don't really understand the question. Are you suggesting we should alter first, then populate the rest of the data? I believe the alter hook should be the very last step to give clients a chance to alter everything. If you move the alter before you set 'tag' and 'show' no one can alter those. Why would you want to prevent that?

Thanks,
-Derek

If i allow to change only the basic data (url/long_url, teaser, short_url, nid, source, title, front_page) any external module doesn't need to override the other derivated tags. In future if i have to add another derivated tag the external module don't need to be updated because it will be automatically generated.

Ok till now don't seem too much work, there are only 14 derivated tags and look enough for all the situations, if someone want to change the url, have to provide 9 tags instead of change 3 values isn't so bad, in D7 is just a question of backward compatibility because the url() function encode the params in its way, but what if we change the way these services are rendered? the external module have to update the single derivated tags too to encode them correctly. So i think is better to leave the core make the encodings and the external module alter only the basic data.

In general i wouldn't allow at all to alter internal settings from external modules if not through the APIs because they could bring more problems than solutions (thinking to this scenario: the module B change the setting X so to have the control on X have to disable or reconfig B, but Service Links core show a different value in its configuration page for X...).

Btw, alter the basic data look interesting for those external module which want to collect statistic info just changing the url as requested here: #1442280: Award userpoints for each share, so we should restrict the altering only to those variables.