[#14269] - Replace dirname(__FILE__) with __DIR__
- Fixed in Code Base
- 3 Apr 2017
- Medium
- Build: 3.8-dev
- # 14269
- Diff
- Spudley:3.8-dev
User tests: Successful: Unsuccessful:
Pull Request for Issue # n/a.
Summary of Changes
This is a simple PR to replace dirname(__FILE__) with __DIR__ across the codebase. I excluded the vendor folder as that's third-party code, but replaced all other instances.
DIR was introduced in PHP 5.3 to reduce the need for convoluted dirname(FILE) constructs.
The change is syntactically identical, but makes the code clearer - it is shorter, easier to read, and easier to understand.
It might also make things slightly quicker, it's one less function to call. But that going to be marginal at best; the real reason for doing this is to make the code easier to work with.
Testing Instructions
This change should have zero impact on the functionality. The new code is functionally identical; just a bit shorter and neater. Testing should just be a case of making sure it still works the same.
Expected result
No change.
Actual result
No change.
Documentation Changes Required
None required.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_joomlaupdate Installation External Library Libraries Unit Tests |
@mbabker: No problem; I hadn't spotted restore.php as being third-party, but fair enough.
Re installation/index.php -- the occurrence of __DIR__ appears after the 5.3 check and won't prevent it parsing, so I don't believe this one needs to be reverted. (I also note that the main /index.php file which has the same requirement already contains references to __DIR__).
Happy to revert, but can you confirm the latter before I do so? Thanks :)
| Labels |
Added:
?
?
|
||
PHP will parse the entire file before executing it, it doesn't parse it line by line.
We should've not changed to using __DIR__ in the administrator/index.php or index.php files for the same reason. It's actually really inconsistent there because there's a comment that explicitly says "use define('FOO', 'bar') instead of const FOO = 'bar'".
| Category | Administration com_joomlaupdate Installation External Library Libraries Unit Tests | ⇒ | Installation External Library Libraries Unit Tests |
| Category | Installation External Library Libraries Unit Tests | ⇒ | External Library Libraries Unit Tests |
Okay. I've reverted it. :)
Looks fine to me.
Whomever merges this, please merge to staging instead of the 3.8 branch. Don't see any reason why it should only apply there.
Bringing what back?
if you recall starting around 1.7 we removed DIR from all of J!.. was a HUGE undertaking and didn't go smoothly. Why are we reverting?
The DS constant was removed. Joomla can't remove __DIR__, it's a PHP language feature.
ah, thats right.. ok.. sorry for confusion
Not able to apply this PR, please see attached screen-shot.
PHP Version : 5.6.30-7+deb.sury.org~precise+1
Web Server : Apache/2.4.25 (Ubuntu)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14269.
Confirming @RonakParmar Comment.
@brianteeman Yes I did. Sorry about that. It was a while ago now! However the patch itself is pretty trivial. If you like I'll re-create it. I guess that'll mean submitting a new PR.
I'm just going to merge by review.
The only change in a productive file is in libraries/fof/view/view.php, all other changes are in unit tests and since those passed it should be fine.
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-04-03 09:58:39 |
| Closed_By | ⇒ | Bakual |
Please revert the changes in these files.
administrator/components/com_joomlaupdate/restore.phpis third party codeinstallation/index.phpneeds to be parseable on PHP 5.2 to display a "your platform isn't supported" message versus a PHP fatal due to the engine being unable to parse the file