I'm using a custom metatag to provide geourl/geodata meta tags.
I'm also using contactinfo and with the provided patch, I'm successfully using lat/lon values entered in contactinfo.
Other contactinfo fields would be also very useful if provided as tokens.
See also: #1278146: [META] Ensure all meta tags are supported (no patches, just planning)

Adding token API hooks and lat/lng tokens by now.

Comments

amontero’s picture

Issue summary: View changes

Link module

dboulet’s picture

Issue summary: View changes
Status: Needs review » Closed (won't fix)

The code looks ok, but I’m going to mark this as won’t fix. If we add tokens for lat/long, then we’ll have to add tokens for all the fields—I don’t think that the need for this feature would be common enough to warrant this additional code. My goal with this module was always to keep it as simple as possible.

Nothing is stopping you from using this code in a separate module, you could easily create a contactinfo_tokens module to add this functionality.

Thanks for the suggestion.

amontero’s picture

As adding code in the main module file and/or keeping it lean and tidy is desirable for you, I can reroll the patch but placing the token functionality in a sepatare contactinfo.token.inc file to take advantage of token module implementing hook_hook_info:
https://api.drupal.org/api/drupal/modules!system!system.module/function/...
This way you'll keep the token functionality out of sight. Also, it will be loaded only when needed, good for performance.

The proposed functionality would just enhance the module without any end-user noticing, but would allow more site devs to do fancy things with it (multisites and distributions come to mind).

I agree on the need to provide the remaing contact fields, too. But creating a new module just for this simple functionality looks overkill to me. Just adding now and placing a "initial token support" banner linking here in the module page would make the module sexier to site devs ;)

dboulet’s picture

Status: Closed (won't fix) » Needs review
StatusFileSize
new5.17 KB

Thanks for the additional info, adding this code to an include file makes sense.

I worked on a new patch which adds all of the tokens, please give it a review to make sure that it meets your needs.

amontero’s picture

StatusFileSize
new5.51 KB

Just two small changes:
* As per UI guidelines, using ' instead of ’ for "site's".
* Changed printing-style double quotes (“Personal”) to regular, since in my non-English browser, they displayed weird chars. Also, as per UI guidelines, regular double quotes are used everywhere.

Here are the patch interdiffs:

18c29
< +    'description' => t('Tokens for this site’s contact information.'),
---
> +    'description' => t('Tokens for this site\'s contact information.'),
23c34
< +    'description' => t('Either “Personal” or “Organization/Business”.'),
---
> +    'description' => t('Either "Personal" or "Organization/Business".'),
27c38
< +    'description' => t('Site contact person’s first name.'),
---
> +    'description' => t('Site contact person\'s first name.'),
31c42
< +    'description' => t('Site contact person’s last name.'),
---
> +    'description' => t('Site contact person\'s last name.'),
83c94
< +    'description' => t('This site’s contact email address.'),
---
> +    'description' => t('This site\'s contact email address.'),

Other than those two, the attached patch is the same which works great in my tests. Thank you very much.

dboulet’s picture

Can you point me to the quideline that recommends using straight quotes? All I could find was the following, which actually recommends doing the opposite:

https://drupal.org/style-guide/content#punctuation

Use curly (smart) quotes unless you are providing code examples.

Smart quotes are generally considered correct, typographically—if they are not displaying properly, then you might need to change the encoding of your page.

More info:

http://smartquotesforsmartpeople.com/

amontero’s picture

StatusFileSize
new5.53 KB

Ops. I was basing #2 in an assumption based on my above referenced and linked page alone. I recall seeing there to use " quotes somewhere. Checked your provided references and put back smart quotes, but using HTML entities this time. They display OK in my browser, now.

Also, bookmarked the nice http://smartquotesforsmartpeople.com/ . Thanks for the link. Since I'm under Ubuntu, here's how to pull smart quotes on Ubuntu, for completeness.

Patches interdiff:

18c29
< +    'description' => t('Tokens for this site’s contact information.'),
---
> +    'description' => t('Tokens for this site\'s contact information.'),
23c34
< +    'description' => t('Either “Personal” or “Organization/Business”.'),
---
> +    'description' => t('Either &ldquo;Personal&rdquo; or &ldquo;Organization/Business&rdquo;.'),
27c38
< +    'description' => t('Site contact person’s first name.'),
---
> +    'description' => t('Site contact person\'s first name.'),
31c42
< +    'description' => t('Site contact person’s last name.'),
---
> +    'description' => t('Site contact person\'s last name.'),
83c94
< +    'description' => t('This site’s contact email address.'),
---
> +    'description' => t('This site\'s contact email address.'),
dboulet’s picture

Do the quotes only render properly for you if you insert them as HTML entities? You shouldn’t have a problem rendering them if you use UTF8 everywhere. Same goes for proper apostrophes.

I’d rather use proper curly quotes and apostrophes, they make the text much easier to read than having to use entities and escaping straight single quotes.

dboulet’s picture

Status: Needs review » Fixed

Thanks again for your help amontero, I just pushed up the changes to add token support to the module.

In the end, I decided to use straight quotes like you suggested, as this is the style used in core modules. I still think that proper quotes are the way to go, but that can be left for the future. Interestingly, I found this old issue suggesting updates to the quote style in core—it wasn’t received very warmly:

#89499: Replace " signs with “curly quotes”

Status: Fixed » Closed (fixed)

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

amontero’s picture

Status: Closed (fixed) » Active

Great to see it added to the module, thanks.
Too bad about the quote issue, now that I was already sold on it.
Minor issue: add this to the project description feature list.

dboulet’s picture

Status: Active » Closed (fixed)