Token 6.x-1.14 breaks location wildcard tokens with Pathauto 6.x-1.4

Same issue. Subscribing.

Hrm, location.module doesn't actually define each token that is possible, hence it messes with token validation. I may have to easy up the token validation to messages instead of errors since other modules can't take the time to define proper tokens.

@ Dave Reid
Thanks for your work on this problem.

Will there be a patch / new version of the token module soon?
Or is it better to go back to Token 6.x-1.13 (is to possible to go back or will there be problems because of changes in the database, ...?)

Subscribing

Bump...

Just investigating this... Location uses a token_name_N pattern for defining tokens with variable results. Is this a common pattern? If so, it seems like something to allow that during validation might be appropriate. Here's a patch against 1.14 (which appears to be the same in 1.x-dev).

Ok, rerolling the patch, from inside the module directory ;-) No actual code difference.

The patch looks good to cover location's N wildcard syntax, but I wonder if that syntax shouldn't be changed before it's standardized across all token-enabled modules. Specifically, I suggest adopting the db_query placeholder syntax, so location-country_N would become location-country_%d.

I don't think it would be easier for users to understand %d vs 'N' since end-users don't know about db_query and its placeholders.

@Dave Reid, sounds like you have a different definition of end users than I do. If you're cool with N as the wildcard, then this is RTBC.

I did have an additional thought about this -- I wonder if the validation code should reject the _N syntax.

For example, Location defines [location-province_N] but that's not actually a valid token -- you need to use [location-province_0]. So I'm thinking that perhaps the validation code should reject [location-province_N] with a message to replace "N" with the appropriate integer... Or is that taking this too far?

@freelock, I think that makes sense, but that does nothing to solve the problem at the beginning of this thread, so it feels like it belongs in a separate issue.

sreynen: I agree, different (but very closely related) issue. I do think this patch should get committed asap, because it's breaking functionality -- my additional proposal is probably more of a feature request/RFC...

Ok, getting back to the root issue, this will fail for any 'token-name-N' tokens as well.

@Dave Reid, I'm confused. Do "-N" tokens already exist? The wildcard pattern in location seems to be "_N", not "-N". Are you suggesting both "_N" and "-N" should work, just in case? That feels like options just for the sake of options, not generally a great idea. Or are you saying that "N" alone should work? Making the wildcard just "N" seems almost certain to result in false positives.

No they don't as far as I know, but I also haven't searched contrib to see if they do. Its not options, it's flexibility. It requires a tiny change to the patch (to support -N and _N), but if we can't do that I can commit the patch as-is.

Updated patch to support both -N and _N versions.

Looks like the patch was targeting a different version. I'm guessing -dev, and changing the issue version to re-test the patch.

#17: token-883804.patch queued for re-testing.

No, just need to get the git diff syntax to work with simpletest in modules ;-) had it figured out for core.. trying again!

Ah, missing --no-prefix.

Looks good to me.

#22: token-883804.patch queued for re-testing.

Looks good to me... a +1 for this getting in! :)

The patch in #22 works great for me... +1

This has been committed to D6 and D5 with additional comments and also supporting the [token-name-?] definition.
http://drupal.org/cvs?commit=424552
http://drupal.org/cvs?commit=424554