adminer
adminer copied to clipboard
vrana
replace functions that cause redeclare errors
These are functions that are extremely common names and have conflicts when including adminer with or via another set of code (example: Laravel)
This example package creates a route in laravel that loads adminer.php https://github.com/leung0826/adminer-for-laravel/blob/master/src/Console/UpdateCommand.php#L119 The command linked to here does a replace to fix the function issue, but it is not very cross platform compatible. There are very few options for doing this replace, none that are cross platform and efficient.
I have combed through many of the plugins for adminer and I have not found any issues with this update.
While I agree with the sentiment of your change, I'm curious as to whether or not you submitted a similar PR to Laravel, and why/why not?
Good question @djmattyg007! I always suggest to everyone that they use verbose and specific function names when writing function based shared/open source code. There is so much conflict that can happen. Laravel has wrapped all its helper functions in "exists" checks. So their code is not causing the issue, technically. But it could cause some major issues.
vendor/laravel/framework/src/Illuminate/Foundation/helpers.php
But they're just as much at fault for using function names that could easily conflict, so you could easily submit a similar PR there too.
Yes, I could / might. Reasons I posted here first...
- Laravel has made an effort to prevent fatal errors due to function declaration
- The functions are helpers, the class methods are the recommended usage.
- They have at least 12 times as many followers as adminer (smaller effect)
- Changing it here effects no known plugins/code.
- Laravel changes would affect numerous packages
Posted issue to laravel/framework for good measure. namespacing and or classes may help here, but this pull request solves the current issue without a major logic change.
Laravel wraps all helper functions in an if block to help avoid collisions. It's a common practice that should be followed in any package that drops functions into the global scope.
the only problem with just wrapping its 'helpers' with "exists" checks, is that the functions can be replaced by code that does something entirely different if mashed up with totally different library. That is why I recommend a unique naming convention (class, namespace, prefix)
One of Adminer development priorities is code size. Compatibility with other libraries is not.
A better solution seems to be to put Adminer into a namespace. But that's going to be tricky with compilation. Also, I maintain compatibility with old PHP versions for a long time.
I'm not sure about the right solution, maybe this pull request is it thus I'll leave it open for now.
@vrana - While I agree with priority of code size, I do not agree with "not concerning" the code with other libraries. This outlook will eventually become detrimental to the survival of this or any other code packages that does not concern itself with compatibility with other packages. I use this package frequently, but would use it even more if it were mashable with other packages.