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

Simon Georges’s picture

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

dww’s picture

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

Thanks,
-Derek

Simon Georges’s picture

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.

dww’s picture

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

Cheers,
-Derek

TheCrow’s picture

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(
    ...
  );
?>
dww’s picture

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

TheCrow’s picture

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.

seanB’s picture

Issue summary:View changes

I would like to add an image from a custom image field to a Tumblr link (using the photo share option).
Having this patch would help me create a custom tag for it to build the URL.

I think it would be great to have more flexibility in the URL's.

seanB’s picture

Status:Needs review» Reviewed & tested by the community