I just went and made a diff of the two checkouts, and, taking D6 specifics into account, here's a patch for 5.x-2.x that should bring the two source tress more into sync. I've only edited out that which I was sure shouldn't be in 5.x, thus there might be more stuff we'd want to rip out.
... Actually, the patch still needs some work and I need a break, so the patch will come later.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | pathauto_5x_6x_sync-264665-12.patch | 1.66 KB | Freso |
| #11 | pathauto_5x_6x_sync.new_.patch | 6.15 KB | Freso |
| #10 | pathauto_5x_6x_sync.patch | 12.7 KB | Freso |
| #8 | pathauto_5x_6x_sync-264665-8.patch | 30.63 KB | Freso |
| #6 | pathauto_5x_6x_sync-264665-6.patch | 32.47 KB | Freso |
Comments
Comment #1
Freso commentedThis still needs a good deal more work, but I'm heading to bed now, so thought I'd post it here so other people might peek at it while I'm away for the next 2-3-4 days. Note that it probably doesn't apply in its current form. I just made a "diff -u" of the two source trees (after cat'ing pathauto.admin.inc onto the end of pathauto.module for 6.x, and have hand-edited it from there. The pathauto.module hunks seriously need to be cleaned up, and the pathauto_node.inc ones do too.
Comment #2
Freso commentedComment #3
Freso commentedHere's the first actual patch. It's not the final though. The only changes to pathauto.module so far have been to move the admin related functions to the bottom/end of the file, for easier comparison with 6.x's pathauto.admin.inc split. Also, there's a hunk in pathauto_node.inc I haven't gotten around to poking at yet.
Comment #4
gregglesThe taxonomy/category stuff shouldn't be merged back to 5.x b/c that is a 6.x vs. 5.x change, right?
Comment #5
Freso commentedI'm honestly not sure, but I'd love to take those bits out of the patch, as that'll mean less strings to (re)translate for the 5.x-2.3 release. :) (And to think I hadn't noticed those numbers before now! Hail Eris!)
Comment #6
Freso commentedOkay, here's a patch without the translation string changes. Greg, could you please review this? I'd really like to at least commit the pathauto.module re-arrangement before I try to work on the actual code changes between the two versions. (Oh, and I'm also rather wary about the JavaScript - I don't even know where it is used or for what, so no clue how to test it either...)
Comment #7
gregglesThe javascript is what disables/enables the path-alias form element on node pages and hides/shows the description underneath it. If you have "create path aliases" permission and create a node and check/uncheck the "Automatic alias" you'll see that in action. After applying the patch, the show/hide of the field description wasn't working for me, so I think we should leave that piece out.
I'm not sure about the changes to _pathauto_path_is_callback - I changed that from 5.x to 6.x due to the new menu API, but if this configuration works in both versions then that seems fine to backport it. The static variable caching is done inside of menu_get_item so we don't need to re-do that if we go this route - makes sense to me.
Other than those items, the rest of it looks good to me.
Comment #8
Freso commentedAlright. I'll be checking the attached batch in in a bit, unless someone is quick to step on and say "STOP!!!". After applying this, I'll look more into the rejected changes previously mentioned, as well as starting to work on some of the stuff I haven't messed with yet.
Comment #9
Freso commentedAlright, batch 1 has gone in. I'll work on sync'ing pathauto.module one of the days.
Comment #10
Freso commentedAlright, here's a new copy of the patch from #1. Hand edited about the same things out (except for some more strings changes that was decided to not include). It should be easy to see how much simpler it is now that batch 1 has gone in. :)
Also, about
_pathauto_path_is_callback(). The code only checks this one place I could find (grep "_pathauto_path_is_callback" *in root dir of 5.x-2.x checkout), and only checks withif (_pathauto_path_is_callback($dst))- ie., the actual return result isn't used for anything, it only cares whether it is or not (), so I believe changing it to be like the 6.x function would work. But I haven't tested it, so...Comment #11
Freso commentedThis one is unlikely to apply, but just putting it up here so you can see the current code that's needed to weed through.
Comment #12
Freso commentedThanks to the power of Jabber and instant messaging, Greg and I just ran over the remaining code differences, and these wee bits are what seems to be in need of sync'ing. :)
Comment #13
gregglesLooks awesome to me.
Thanks, Freso!
Comment #14
Freso commentedThat was the last to go, and it is now gone. 6.x-1.1, 'ere we come!
Comment #15
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.