Fix pathauto_cleanalias() and pathauto_cleanstring()

I'm inclined to commit this but leave it here for review for a few days.

Regarding the new hook and why scope creep is ok: It will allow us to say "oh, you want to do crazy? yeah, do it yourself based on hooks." Instead of just arguing over what we are/aren't going to do.

Hi,

I'd run into a similar issue and was about to add a feature request/patch.

My alias pattern is something like this:

1/[field_name-raw]-[field_optional1-raw]-[field_optional2-raw]

I had been putting whitespace in the pattern, instead of the separator, and then had a small patch to pathauto.inc to trim and replace whitespace in the complete alias.

Now I've tested the patch in #1 instead and it works well for me. :)

Thanks dww for this patch.

Applied to 6.x-1.x http://drupal.org/cvs?commit=327030

@dww Any chance you can port this to 7.x?

D7 patch attached for review.

Revised and committed to 7.x-1.x and 6.x-2.x as well.

Something I'm not exactly sure on is why the lowercasing is in a function meant to remove separators. Also, we need to document this in pathauto.api.php.

This inconsistency needs to be fixed before the next release on any branch.

I'm really not happy with the hook that was added here.

1. I don't think it makes sense to be invoked in the context of pathauto_clean_alias(). The purpose of this is to alter the alias that pathauto is generating, then it should be renamed to hook_pathauto_alias_alter() and be invoked from pathauto_create_alias().
2. Lowercasing was moved from pathauto_cleanstring() to pathauto_cleanalias(). Again does not make sense.
3. If we really wanted a pathauto_cleanalias() then the rest of the string cleanup functions from pathauto_create_alias (e.g. removing duplicate repeated slashes, trimming leading or trailing slashes) should have been moved there as well.
4. Any kind of 'url' tokens should not have pathauto_cleanstring() applied. It should always be assumed that if URL tokens are used in a pattern we only want to check for duplicate slashes around alias once the token has been replaced.

With a test creating a parent term with the alias 'My Crazy/Alias' and a child term with the pattern [term:parent:path]/[term:name], so it would end up as 'My Crazy/Alias/child-term'

Great, we found a core bug: #870528: taxonomy_get_parents(), taxonomy_get_children(), and taxonomy_get_tree() do not return a full term objects.

Still hoping that core bug gets fixed shortly, but don't think I can do much until then. :/

#17: 545216-pathauto-fix-cleanalias-D7.patch queued for re-testing.

Let's try that without the debugging line and a stupid logic error in the test...

#23: 545216-pathauto-fix-cleanalias-D7.patch queued for re-testing.

Did a contrib-wide search for implementations of hook_pathauto_clean_alias and found none. Committing shortly.

Committed to 7.x-1.x CVS. http://drupal.org/cvs?commit=404100

Try this again with PHP 5.3 fixes...

Simpletest for D6 is getting really irritating with not installing dependencies. This passes absolutely fine locally.
Committed to 6.x-2.x: http://drupal.org/cvs?commit=404400

Committed to 6.x-1.x as well after manual testing: http://drupal.org/cvs?commit=404428

Sorted ?