Use drupal_str functions instead of str functions
Freso - May 23, 2008 - 17:11
| Project: | Pathauto |
| Version: | 5.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Pathauto is currently using a bunch of strtolower(), strlen(), and other string functions that have drupal_ equivalents. We should probably use the drupal_ version for most (if not all) of these instances. (If someone gets around to it before I do, here's a hint: Use Coder to find the instances. :))
I'm also going mark this as blocking the release of 6.x-1.1.
#1
Is there a reason these instances were not using the
drupal_functions? (Attached patch has not been tested.)#2
No reason that I know. I think I just didn't know about the drupal_*str* set of functions. This seems RTBC to me.
#3
I gave this some testing and, though I found some other bugs, I don't think this introduces any bugs (my testing and the simpletests still work as expected).
#4
Alright then, committed. :) It should probably be backported though.
#5
The patch more-or-less applied to 5.x (one failed hunk, some offset), so committed there as well.
I also stumbled upon an empty
for-loop while applying the failed hunk:<?phpfor ($i = 0; _pathauto_alias_exists(drupal_substr($alias, 0, $maxlength - drupal_strlen($i)) . $separator . $i, $src); $i++) {
}
?>
This should probably be investigated for another (clean-up =)) patch... (It exists in both D5 and D6 versions.)
#6
I think the empty for loop is just a way to keep trying new aliases until we get one that's unique. It's perhaps an overly fancy/brief construction which is difficult to understand (I didn't write it and I still have to think about what it does to see if it works).
#7
From what I can see, nothing is done during the loop - not while calculating the criteria, not while actually running it! If it does anything, it's using time/CPU cycles. But then, perhaps you're seeing something I'm not. ... And then I realised it's making and storing a count in
$i, which is used just a bit later... This is definitely worthy of a code comment. Before we forget what it is. :)#8
And since
$iwill always be an integer, we can safely use PHP's ownstrlenon it, for speed. So I'll reverse this bit.#9
Automatically closed -- issue fixed for two weeks with no activity.