bcit-ci / CodeIgniter

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
18.27k stars 7.59k forks source link

Not able to overload the error 404 override controller routiing #3552

Closed wiredesignz closed 9 years ago

wiredesignz commented 9 years ago

Having the routing code which locates the error 404 override controller in the core Codeigniter.php file rather than the Router class prevents overloading the method and limits functionality.

Recommend the code is relocated to the Router class where it functionally belongs.

narfbg commented 9 years ago

The same code spread around between the bootstrap and CI_Router wasn't better ... Why would you need to override it?

wiredesignz commented 9 years ago

Why do you ask why? The answer is because it should be possible!!.

One of CodeIgniter's best features is the ability to overload and change it's core functionality.

The method used to locate the 404 override controller is a duplication of the code in the Router class used to located the default controller.

Both of these items are functionally equivalent and you can overload the default controller detection but not the 404 override controller. I think the real question is why do you not allow it?

narfbg commented 9 years ago

The relevant code in CI2 had a lot more duplications and the inability to overload the 404_override logic is a side effect of reducing those duplications and the removal of methods that were always supposed to be protected, but were instead public because it's otherwise impossible to call them from the bootstrap file.

It's not a matter of why do we not allow it, it's a matter of why an extra effort should be made to allow it. And my number 1 rule is that "why not" is never a reason enough, so you'll have to be more convincing than that ...

wiredesignz commented 9 years ago

Comparisons to CI2 are not relevant you've made that point clear in the past yourself.

You're ignoring the fact that CodeIgniter has always allowed it's core files and functionality to be overloaded and you are compromising that feature by placing the 404 controller detection code in the Bootstrap file.

I don't see how Router method visibility is an issue when it already has other public methods accessed from the Boostrap file? . Isn't it also bad design to have related functionality scattered across different files. The default controller is functionally equivalent to the 404 override controller and is located in the Router.

In any case I think you should get another opinion on this maybe ask Mr Parry before simply forcing your ideas onto the community. @Narfbg

narfbg commented 9 years ago

The fact that I've left this issue open shows that it's not a done deal and I don't appreciate you stating that I'm forcing my ideas about this to anybody.

I'll ask again - why do you need to do that? Please give me an actual use case and not bullshit rhetoric ... you're even contradicting yourself with the very first two sentences that you wrote.

wiredesignz commented 9 years ago

There's no need to lower the conversation to using profanity @Narfbg. It only goes to show that you have no other sound argument to offer. The fact that you have such a condescending attitude towards people both here and on the forums does the new owners of the framework a huge disservice. @jim-parry

I don't see any contradiction and I have explained twice now that you are reducing the core functionality and introducing poor design by having the 404 override controller detection code in the Bootstrap file rather than the Router where it belongs.

[Edit] Also I have just noticed that it is not possible to move the "fallback" 404override controller to a different directory with the current code in the Bootstrap.

Just get another opinion on this please before you simply shut this topic down. Thanks.

narfbg commented 9 years ago

Oh, I'm lowering the conversation?

Am I the one ignoring a simple question about a practical use case 3 times in a row? Am I the one using personal attacks in this conversation? Am I really the one being condescending here and now?

Get a grip man, I can often be rude, but you're an undisputed champion in that regard. Last time I had an exchange with you (that I know of), I received the following PM from another user:

Ignore the guy.

He was a dick, he is a dick, and he will always be a dick. He trolls in IRC as well, popping into channels of other frameworks.

In #*** we have a permanent ban for him…

If you want to talk about sound arguments, just answer the one simple question that I asked you and that is really the only thing important in this discussion - why the fuck do you need to overload the 404_override logic?

Yes, my changes have reduced functionality and just as you've explained that, I've explaned why these changes were made.

Also, here's your contradiction:

Comparisons to CI2 are not relevant you've made that point clear in the past yourself.

You're ignoring the fact that CodeIgniter has always allowed it's core files and functionality to be overloaded and you are compromising that feature by placing the 404 controller detection code in the Bootstrap file.

And by the way - there already was 404 controller detection code in the bootstrap, my changes just made restricted-access code even more restricted, so don't bullshit me about introducing poor design.

Assuming that you really care if any change will happen (because quite honestly, I question that), you're either ruining it for everybody or just butthurt because you'll have to change more than 5 lines of code in your HMVC extension.

I've got nothing against re-introducing the same functionality, but if you want that to happen, be a grown-up for a minute and give me and everybody a damn use case. Until then, I can call you LongtimeUser4 and not give a shit about it.

And of course, somebody, somewhere is having fun reading this, so here's a bonus: shit, piss, fuck, cunt, cocksucker, motherfucker, tits ;)

wiredesignz commented 9 years ago

Wow!! Andreev you need to calm down. You're not being very professional here. Why do you feel the need insult and run people down who you disagree with you. What happened 2 years ago between us has no bearing on this subject. I suggest you take the stick out of your arse and stop being so precious. @jim-parry. Is this the best you can do for a lead developer?

[EDIT] Re: the contradiction. CodeIgniter has allways allowed its core functionality to be ovelroaded this has nothing to do with any particular version it is a fundamental feature.

[EDIT] As far as Modular Extensions goes I already have a working codeigniter-3.x branch but I also wish to relocate the fallback controller to a different location which I cannot do. at present.

All of your ranting doesn't help, you really do need to grow up. @narfbg

ZombieProtectionAgency commented 9 years ago

@jim-parry. Is this the best you can do for a lead developer?

Sorry to jump in but.... @wiredesignz I don't think trying to tattle on him is very nice. if you look at the commit history for codeigniter you will see that narfbg has pretty much single handedly been keeping codeigniter alive and updated for the last few years. He deserves some slack, if nothing else.

wiredesignz commented 9 years ago

@ZombieProtectionAgency Sometimes when a person is behaving erratically or inappropriately it's necessary to let someone else know what's going on. @jim-parry

@ZombieProtectionAgency As far as Narf being the lead developer for so long I believe he thinks he owns the code and has become very precious about it.

narfbg commented 9 years ago

Oh, I am very much calm. :)

In fact, I've been undeservingly polite to you up until now, despite your cocky attitude. The inevitable just happened - you being a dick - and somebody had to give you some of your own medicine for a change.

Not surprising, you're still behaving like in a kindergarten ... Jim Parry receives all of this even if you don't "tell on" me.

wiredesignz commented 9 years ago

I expected nothing less from you @Narfbg even before I posted this issue yesterday I thought that your arrogance and ego would cloud the discussion.

[STAYING ON TOPIC] I have given my use case in the edits posted above and explained my reasoning a few times already.

narfbg commented 9 years ago

Speaking of expectations ... you brought up the past, not me.

But back to the present, if you're referring to this as your use case:

[Edit] Also I have just noticed that it is not possible to move the "fallback" 404override controller to a different directory with the current code in the Bootstrap.

Then the answer is very simple - yes, it's not possible; moving to another directory was never the intended behavior.

Edit: This @Narf person got subscribed to read our shit by a triple mistake ... claims the nickname before me pretty much everywhere, weird way to get introduced. :)

wiredesignz commented 9 years ago

There was never an intended behaviour.

CodeIgniter has never forced its users to follow any particular design pattern. That was a foundation philosophy of Rick Ellis when he first created CI. In its original form it didn't even have a model class.

Who decided when this philosophy would be changed?

narfbg commented 9 years ago

Well, that's a "solid" argument ... how does "never an intended behavior" support your request? A BC break is the closest thing to a valid point that you've shown so far.

The fact is, no piece of documentation (including config file comments) has ever suggested that a controller directory was configurable. That it has ever worked is an incident enabled by sloppy coding and the intention to allow configuring controller/method pairs. In other words - it was a bug, not a feature.

This has nothing to do with philosophy and you know it.

wiredesignz commented 9 years ago

if you already intentionally allow the Router to be overloaded which includes controller detection and the default controller location then it can't be a bug. The controller directory path is able to be modiifed. Adding the fallback 404 override controller to this mix adds to the core flexibility it does no harm.

narfbg commented 9 years ago

I've never said that it does any harm, nor that it doesn't add flexibility. Never even implied either of these.

And in the context of CI_Router - yes, changing the directory is possible and unless I'm mistaken, it is possible for 404_override as well. But this is where it gets a bit blurry because you're mixing the two ...

This is in fact a new feature - default_controller and 404_override no longer ignore the CI_Router::$directory property. They are now more dynamic, per directory values that allow you to have i.e. a default controller for the wwwroot directory and a separate default controller for admin/ ... The limitation being that both must be named identically, say 'Default', only residing in different directories.

Makes a lot of sense when you think about it - if you go to example.com/foo/, it will try to load the index file for foo/.

The bug consisted of the configuration values themselves being able to include a directory - doing an explode('/', $var) on either of the configuration values and not checking which side of the slash is what, resulting in the 404_override scenario always being executed and in turn re-running the whole routing procedure. That is of course just how I remember it and not necessarily an exact description - the point is, it was really, really messy code and it was obvious that what you see as a feature was in fact a freakish accident.

wiredesignz commented 9 years ago

A per directory default controller makes sense so I've modified Modular Extensions to mimic that behaviour. Which in turn has enabled use of a per directory 404override controller.

Unfortunately the fallback 404override controller is still going to be hard coded into the application controllers directory. So be it.

It's a pity that this discussion got personal. @narfbg

ivantcholakov commented 9 years ago

@wiredesignz I afraid that you are going to make a kind of compromise with your HMVC implementation.

Long time ago I adapted your HMVC 5.4 for CodeIgniter 3 and I found that it is impossible without modifications of some CodeIgniter's files. I am proposing you the idea you not to release a separate add-in with installation instructions, but a "flavored" CodeIgniter 3 with HMVC 5.5. Thus, you can hack whatever you want and you will not have to make unpleasant decisions. Also you may do the same with the next CI 3.x versions, this approach is future proof.

Otherwise, even if you negotiate compatibility now, it would be for this moment only.

narfbg commented 9 years ago

@wiredesignz

I see you've made edits to your comments, some of which not marked as such, and some make parts of my own comments look quite puzzling (specifically, you had mentioned @Narf 3 times, while at present it's only my comment that has it ...) - that's not particularly nice.

You've also included more personal attacks, while your last comment pretends that you don't like it that this conversation became personal.

The following was not part of your original comment:

@ZombieProtectionAgency As far as Narf being the lead developer for so long I believe he thinks he owns the code and has become very precious about it.

And neither was this:

All of your ranting doesn't help, you really do need to grow up. @narfbg

... all notifications are kept in my inbox.

I assumed were done with the personal attacks, but I guess that's just not in your nature. As much as you like to pretend that I'm the bad guy and the one acting childish, I only became rude after you personally attacked me twice and you never explained your use case until after that (and that was another issue in the first place).

So, from now on, I'm just assuming that you never had anything constructive in mind and I'm closing this issue on the basis of that. If you ever intend to participate in something here, please make it with a well-explained pull request instead of empty criticism.