bcit-ci / CodeIgniter

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

Error pages - user guide and i18n improvements, custom title #905

Closed hansfn closed 10 years ago

hansfn commented 12 years ago

User guide

Ref http://codeigniter.com/user_guide/general/errors.html

show_error('message' [, int $status_code= 500 ] )

should be replaced with

show_error('message' [, 'status_code' = 500 [, 'heading' = 'An Error Was Encountered'] ] )

and the text updated.

Custom title

Currently you can set the heading and the message for error pages, but not the title. The title is hard-coded:

To me it's OK to use the heading as title so we don't have to change the signature of the show_error function.

Translation

The heading and message strings aren't translatable for the 404 page:

        function show_404($page = '', $log_error = TRUE)
        {
                $heading = "404 Page Not Found";
                $message = "The page you requested was not found.";

I think it would be an improvement if these two strings were translatable.

brodkin commented 12 years ago

I like the idea here, but this seems to create too many issues with backwards compatility for not enough gain. Ths single change would not only break many applications, but it would also break many libraries, packages, and sparks.

I'd throw my support behind this if you'd consider keeping the existing 2-parameter format. I'd suggest allowing the first parameter to be a string or an array containing the header, message, and template. Passing configuration information in an array would be consistent with the way that configuration data is passed when loading libraries in CI currently.

hansfn commented 12 years ago

@brodkin Maybe you misunderstood my request? I'm not asking for an update to show_error, it currently takes 4 parameters - check system/core/Exceptions.php. I'm just asking for the docs to reflect the actual function signature.

philsturgeon commented 12 years ago

This seems like a pretty valid point. @hansfn I'd like to avoid sounding like a lazy bugger, but would you send in a pull request with these changes?

I know everyone is busy but it's the best way to see problems solved. You see a problem, you fix it, everyone wins.

brodkin commented 12 years ago

@hansfn Sorry, that was my ignorance. Please accept my apology.

@philsturgeon What's going on here in that case? Is CI determining its behavior based on the number of parameters or is the user guide just out of date?

philsturgeon commented 12 years ago

I am simply point out that if something is wrong a simple pull request is often better, as it shows exactly what is trying to get done. Taling about code is tough, but changing something then pointing at it is WAY easier.

brodkin commented 12 years ago

@philsturgeon Agreed. I'm just wondering if you're familiar with the current implementation of the function.

philsturgeon commented 12 years ago

I can read code, sure!

function show_error($message, $status_code = 500, $heading = 'An Error Was Encountered')

In Common.php we have this function, while the exception method is different:

function show_error($heading, $message, $template = 'error_general', $status_code = 500)

I'm just pointing out that the poster needs to make a better case for what is trying to be done.

brodkin commented 12 years ago

@philsturgeon Alright, smart ass. ;)
I realized that I could just go check and was trying to locate the function right now.

So where does this leave us?

The documentation is just missing the option to set a header as a third parameter?

philsturgeon commented 12 years ago

Let's wait for the OP to answer.


Ryan Brodkin mailto:reply@reply.github.com 25 January 2012 09:54

@philsturgeon Alright, smart ass. ;) I realized that I could just go check and was trying to locate the function right now.

So where does this leave us?

The documentation is just missing the option to set a header as a third parameter?


Reply to this email directly or view it on GitHub: https://github.com/EllisLab/CodeIgniter/issues/905#issuecomment-3654203

hansfn commented 12 years ago

Yes, the first part of the issue is just about updating the documentation with the third parameter. (I had quoted the wrong show_error function - sorry. I have updated the initial post.)

brodkin commented 12 years ago

@hansfn Perfect. Are you able to submit a pull request with the change?

narfbg commented 10 years ago

The docs have already been updated and the other part is rarely useful. If you really need to alter the title though - the error templates are under the application directory, which means that you're not only free, but encouraged to alter them to your liking. In fact, they've been moved under application/views/errors/ so they are also considered to be views - make your own. :)