37 replies to this topic

[mindplay.dk]

I tried to integrate parts of another site with ProcessWire today, and quickly gave up, as my existing (older) site has a bunch of classes in the global namespace that collide with some very generic class-names in ProcessWire: Database, Config, Session, Page and so forth.

Would you consider introducing support for namespaces? Perhaps in ProcessWire 3.0, as clearly this would break backwards compatibility with the current API.

Simply placing PW classes in a vendor-namespace (e.g. "wire") would probably be sufficient to avoid 99% of collisions.

I think it would be great if PW-modules followed the "{vendor}\{package}" namespace convention for modules, as this eliminates practically all namespace collisions.

Both conventions are compliant with the PSR-1 recommendation - though vendor-name is the minimum required, but I think that's fine for the framework itself.

A "smart" IDE like PHP Storm could probably refactor most of the codebase to use namespace more or less automatically. If you're interested, I also have a script somewhere that rewrites entire codebases to namespaces, picks up folder-names and maps those to namespaces as configured. (it's not complete, but would probably get you roughly 97¹/₄% of the way...)

[apeisa]

Ryan has mentioned namespaces few times already, they are definitely on the long term roadmap. But as long as we want to keep PW compatible with php 5.2, then no hope for namespaces.

Very interesting that script you mentioned. Have you tried it on current PW codebase?

[mindplay.dk]

Very interesting that script you mentioned. Have you tried it on current PW codebase?

I just gave it a go - yeah, it seems to work.

As said, it's not finished - looks like I still need to add support for the "implements" keywords for it to fully migrate scripts correctly.

Some other minor issues with PW is it uses functions - I don't have any support for functions, just classes. Shouldn't be too much work to do those by hand though, only a handful of functions in there as far as I can tell.

It also expects a 1:1 mapping between filenames and classnames... PW seems to break that convention in a few cases, e.g. "core/Array.php" actually contains a class "WireArray" - I'm not sure why that is...

I will tinker with this a bit more and post my results.

[mindplay.dk]

Alright, I added support for the "implements" keyword and tweaked/improved a few other things.

The code is here.

The issues described above (as relates to the PW codebase specifically) still persist - I can't (won't, shouldn't) fix those, but they should be easy to fix by hand.

This is currently set up for testing - it's "non-destructive", at the moment it won't write any files, just lets you preview the changes on a file-by-file basis, so we can evaluate whether this looks correct, or whether this approach is feasible to begin with.

Let me know what you make of this?

EDIT: posted some additional updates and improvements.

By the way, currently the core classes are namespaced as "wire\core" and modules as "wire\modules"... this is simply due to the folder-structure. I started tinkering with a "remapping"-feature that would allow you to override the mapping of a folder to a particular namespace, but quickly realized that was a total waste of time, since you can just rename the actual folders. In other words, to change the namespace-hierarchy, just move files around or rename folders as needed.

But my guess is, we're already happy with the folder-structure of WP? Makes sense to me anyway. I think it would be fine to separate the core from the modules entirely.

Also note, in the last update I turned of namespace capitalization. I used to argue that a namespace is a name, in the same sense that a class or interface is a name, and therefore all should be capitalized. But lately I've changed my mind - unlike a class, interface or trait, a namespace is not a type, and I prefer the visual distinction between namespaces and actual types...

EDIT: another update - the script now reports inconsistent class/file-names, and produces a warning when multiple types are declared in the same file. I would prefer to see PW3 comply at least with the PSR-0 convention of one type (class/interface) per file. And use regular *.php file-extensions rather than the esoteric *.module extension - smells like Drupal. If you must use a magical file-extension, it's better practice to use a double file-extension, e.g. "foo.module.php", designating that this is in deed still a PHP file, with a special purpose, and preventing accidental loading by an auto-loader.

On the namespace issue, consensus among those I have spoken to seems to be, both camel-case and all-lowercase is equally acceptable.

Edited by mindplay.dk, 16 June 2012 - 09:06 PM.

[porl]

Really interesting developments. Is there somewhere with information on the reasoning for sticking to PHP5.2 compatibility? I can understand that bug fix releases etc need to keep it as such, but would it not be holding things back in the long run to keep supporting 5.2 as new features are added?

[diogo]

@porl Not a very extensive information, but Ryan explains it here http://processwire.c...-22/#entry12450

[mindplay.dk]

Makes sense. In my opinion, breaking backwards compatibility (with modules, as well as with the language itself) in something that should only happen with major releases of any platform.

Major releases should target new development, while minor releases should target maintenance and provide an upgrade-path. For major releases, in my experience, providing an upgrade-path is "nice to have", but not "must have", as major upgrades very rarely actually happen in reality.

Anyways, I'm sidetracking...

[mindplay.dk]

I wonder, should I finish that script? Any interest? (Ryan?)

I was thinking I might have it actually rewrite the code to a separate output folder, and (perhaps optionally) break classes into separate files, so that they can autoload without a class-map.

Is this useful?

[ryan]

Definitely interested. We do get occasional questions about how to use ProcessWire with some other framework or CMS, and they are running into name conflicts with some and not others. I think most recently I heard Laravel has some class name conflicts with PW, but CodeIgniter doesn't. So I think there would be demand for something that provides a solution to those conflicts. My only worry is if this makes a user's upgrade path difficult, since they are running on a modified version.

[mindplay.dk]

That's why I'm suggesting namespaces for version 3 - clearly there is no way changing all the class-names to using namespaces is going to be backwards compatible.

Though on the other hand, I don't suppose it's unthinkable you could actually write out the class-name mappings from my tool, and derive an upgrade-tool that would rewrite third-party extensions and applications the same way it rewrites PW...

I personally would worry less about upgrade paths - in my 14 years of experience, upgrading an existing application to a new major release of a framework is very, very rare. For the most part, it just doesn't happen - and major revisions should not sacrifice major improvements to support 5% of the applications that upgraded; most likely 95% of applications will be new applications...

[mindplay.dk]

I just fixed a bunch of issues in the conversion script and added buttons to convert/revert - I believe it's closer to finished now.

[sevfurneaux]

Hi Mindplay,

I attempted to use your script however am having issues. I dropped it into the root folder but it seems to break the site (site is then blank). Here is what warnings I get when I first run it:

Warnings
class-name "WireArray" is inconsistent with file-name "wire/core/Array.php"
class-name "Breadcrumb" is inconsistent with file-name "wire/core/Breadcrumbs.php"
File "wire/core/Breadcrumbs.php" contains 2 type-declarations
class-name "WireData" is inconsistent with file-name "wire/core/Data.php"
class-name "WireDatabaseException" is inconsistent with file-name "wire/core/Database.php"
File "wire/core/Database.php" contains 2 type-declarations
class-name "WireException" is inconsistent with file-name "wire/core/Exceptions.php"
class-name "WirePermissionException" is inconsistent with file-name "wire/core/Exceptions.php"
class-name "Wire404Exception" is inconsistent with file-name "wire/core/Exceptions.php"
File "wire/core/Exceptions.php" contains 3 type-declarations
class-name "FieldgroupsArray" is inconsistent with file-name "wire/core/Fieldgroups.php"
File "wire/core/Fieldgroups.php" contains 2 type-declarations
class-name "FieldsArray" is inconsistent with file-name "wire/core/Fields.php"
File "wire/core/Fields.php" contains 2 type-declarations
class-name "WireInputData" is inconsistent with file-name "wire/core/Input.php"
class-name "WireInput" is inconsistent with file-name "wire/core/Input.php"
File "wire/core/Input.php" contains 2 type-declarations
class-name "InputfieldHasArrayValue" is inconsistent with file-name "wire/core/Inputfield.php"
File "wire/core/Inputfield.php" contains 2 type-declarations
class-name "InputfieldsArray" is inconsistent with file-name "wire/core/InputfieldWrapper.php"
File "wire/core/InputfieldWrapper.php" contains 2 type-declarations
class-name "Saveable" is inconsistent with file-name "wire/core/Interfaces.php"
class-name "HasRoles" is inconsistent with file-name "wire/core/Interfaces.php"
class-name "HasLookupItems" is inconsistent with file-name "wire/core/Interfaces.php"
class-name "TrackChanges" is inconsistent with file-name "wire/core/Interfaces.php"
class-name "FieldtypePageTitleCompatible" is inconsistent with file-name "wire/core/Interfaces.php"
File "wire/core/Interfaces.php" contains 5 type-declarations
class-name "ConfigurableModule" is inconsistent with file-name "wire/core/Module.php"
File "wire/core/Module.php" contains 2 type-declarations
class-name "Notice" is inconsistent with file-name "wire/core/Notices.php"
class-name "NoticeMessage" is inconsistent with file-name "wire/core/Notices.php"
class-name "NoticeError" is inconsistent with file-name "wire/core/Notices.php"
File "wire/core/Notices.php" contains 4 type-declarations
class-name "NullPage" is inconsistent with file-name "wire/core/Page.php"
File "wire/core/Page.php" contains 2 type-declarations
class-name "ProcessController404Exception" is inconsistent with file-name "wire/core/ProcessController.php"
class-name "ProcessControllerPermissionException" is inconsistent with file-name "wire/core/ProcessController.php"
File "wire/core/ProcessController.php" contains 3 type-declarations
class-name "WireSaveableItems" is inconsistent with file-name "wire/core/SaveableItems.php"
class-name "WireSaveableItemsLookup" is inconsistent with file-name "wire/core/SaveableItemsLookup.php"
class-name "SelectorEqual" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorNotEqual" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorGreaterThan" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorLessThan" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorGreaterThanEqual" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorLessThanEqual" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorContains" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorContainsLike" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorContainsWords" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorStarts" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorEnds" is inconsistent with file-name "wire/core/Selector.php"
class-name "SelectorBitwiseAnd" is inconsistent with file-name "wire/core/Selector.php"
File "wire/core/Selector.php" contains 13 type-declarations
class-name "TemplatesArray" is inconsistent with file-name "wire/core/Templates.php"
File "wire/core/Templates.php" contains 2 type-declarations
class-name "WireUpload" is inconsistent with file-name "wire/core/Upload.php"
class-name "CommentFormInterface" is inconsistent with file-name "wire/modules/Fieldtype/FieldtypeComments/CommentForm.php"
File "wire/modules/Fieldtype/FieldtypeComments/CommentForm.php" contains 2 type-declarations
class-name "CommentListInterface" is inconsistent with file-name "wire/modules/Fieldtype/FieldtypeComments/CommentList.php"
File "wire/modules/Fieldtype/FieldtypeComments/CommentList.php" contains 2 type-declarations
class-name "PagerNavItem" is inconsistent with file-name "wire/modules/Markup/MarkupPagerNav/PagerNav.php"
File "wire/modules/Markup/MarkupPagerNav/PagerNav.php" contains 2 type-declarations

Be great if I could get this working.

Thanks

[mindplay.dk]

Well, the script as such works - the problem is, there's a number of inconsistencies in the ProcessWire codebase itself... that's what all the warnings are about - these are issues you can't automate a script to fix.

Long story short, the codebase needs some updates to conform with PSR-0 for autoloader interoperability.

And, after running the script, some things will need to be tweaked manually - for instance, spl_autoload_register('ProcessWireClassLoader') will no longer work because it's now namespaced, so you'd need spl_autoload_register(__NAMESPACE__.'\ProcessWireClassLoader') instead.

Anything involving strings would need to be fixed after running the automated namespace-migration.

In summary, it's only really feasible to do the broad strokes using an automated script - this script isn't for end-users to convert ProcessWire (or any other codebase) on demand, it's a tool to help maintainers update their codebases to use namespaces...

[mindplay.dk]

A few more tweaks today, and apparently a bit more work to do still - the script was not handling built-in PHP classes and interfaces correctly, and still doesn't handle it completely optimally, but better.

Note that the script is now a lot slower - it may be very slow on your machine, depending. This is because the script now takes two passes already during the analysis, in order to provide more warnings...

[mindplay.dk]

@Ryan: running the script now should give you a pretty clear overview of things that need to be corrected to comply with PSR-0: correcting filenames and breaking classes/interfaces into separate files.

I think these can be implemented on the current branch, without breaking backwards compatibility? (with some simplification of the autoloader.)

[mindplay.dk]

@Ryan: you liking my last post, should I take that as "yes, I agree"?

If so, I might fork PW and take a stab at the clean-up round myself. I don't want to spend a lot of time on it though, if you're already doing it, or if you have reservations about breaking types into separate files for some reason...

(what's considered good practice? fork, modify and pull-request - or fork, branch, modify and pull-request?)

[ryan]

@Ryan: you liking my last post, should I take that as "yes, I agree"?

I thought it sounded good, but haven't yet done enough research to answer definitively. I'm not sure that I understand the full scope of changes it would involve yet. But the benefit of meeting that standard would seem to answer any reservations about breaking things into yet more files, assuming it doesn't add overhead to the system. I think it's generally safe for files in /wire/core/, but some concerns about breaking stuff down in /wire/modules/ because there are possible file location dependencies in there (which can be overcome I'm sure).

(what's considered good practice? fork, modify and pull-request - or fork, branch, modify and pull-request?)

I'm probably not the best person to outline best practices about forks and pull requests. I just know the way that I understand code is a little different from others. I don't have a high comfort level with automated code changes from Git, because it's too easy for me to gloss over the details. I generally need to type something myself before I really see and understand the full scope of it. So the way I handle pull requests is to review the changes and basically re-type (or copy/paste where comfortable) into the code and commit them myself. I still tag it as the pull request so that GitHub understands it as the original pull request. This probably sounds a bit insane, but what can I say, I'm old and it works, and it helps me spot any issues a lot sooner. So if you are interested in making these changes, it doesn't matter to me if you do a pull request or outline the changes here in the forum, email, etc. Whatever works best for you is good with me.

Thanks,
Ryan

[mindplay.dk]

Yes, there is a small performance-impact by using more files - that's why I asked, some people are hysterical about 0.1 msec of overhead on a page-load. On a server that uses a bytecode-cache, the impact of a few more files has no significant impact on performance. On the contrary, if we were to package the core-classes in a phar-archive, individual classes and interfaces would actually load slightly faster than they do now.

Reviewing and tweaking changes made to your codebase by third parties doesn't sound like a bad idea at all - actually, I find it reassuring that you're that careful.

I am not a Github wizard myself, but I was recently corrected when I forked and posted a pull-request without branching - supposedly branching makes it easier to merge, I don't completely understand why, but you don't seem to care anyhow, so...

I can see how the impact on modules is potentially harder to predict, so I'll stay away from that - let's take one thing at a time. I'll take a look at fleshing out the core files and possibly packaging them as an archive, tweaking the auto-loader etc...

PS: Just for the heck of it, could you point me to the class/method that enumerates modules? I'd like to take a quick look and assess the potential complications.

[mindplay.dk]

Alright, the first pass for "wire/core" was pretty easy - have a look.

This doesn't seem to break anything - fortunately PW doesn't have a million admin-screens, so I clicked through all of them, and everything appears to be working normally.

If you'd like, I can send a pull-request?

I wonder if I should branch again before starting some of the more "dangerous" work...

[mindplay.dk]

@Ryan there is one small problem with your approach of re-implementing all the changes...

I can't actually continue with the more complex changes - because the next set of changes will be a branch based on the first set of changes.

Are you willing to consider pulling? Otherwise, basically I can't really continue until you review and commit the same changes to your master-branch, as your changes would not be (byte for byte) identical to mine, and thus you would not be able to merge my next branch... I would need to wait for you to implement these changes and then branch from your master-branch again before implementing the next set of changes...

PS: if you grab the latest version of "migration.php" from above, you can see this brings down the number of warnings considerably - there is also a list of notices output by the latest version of this script, which can help identify include/require statements that need to be reviewed...