Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Added option to return sub-controller output #560

Closed
wants to merge 2,502 commits into
from

Conversation

Projects
None yet
Contributor

dchill42 commented Oct 11, 2011

This new feature makes minor additions to the following files:

Output.php:

  • Adds fork_output() to divert captured output to a separate buffer (forked_output)
  • Adds get_forked() to retrieve the diverted output and end output forking (or not)

CodeIgniter.php:

  • Adds $return variable to call_controller() for capturing and returning sub-controller output via new fork_output methods

Loader.php:

  • Adds $return variable to controller(), which is passed on to call_controller() (if $call is TRUE), resulting in return of output instead of TRUE (on success)
Contributor

philsturgeon commented Oct 11, 2011

Why forked? Seems like a funny name for it. Also, would it be able to support multiple nested requests, or just the one?

People like to do crazy stuff with HMVC so we need to have it ready for these things or get a whole barrage of "Why doesn't it do X Y and Z?" spam.

Contributor

dchill42 commented Oct 11, 2011

Well, specifically because it's a funny name. All the output is already buffered, so that wasn't any distinction - this was just the first thing that came to me.

You do have an excellent point about nesting - it would be much more robust if we could nest the buffers, mirroring what the ob_ calls do. I can lose the flag and turn the original buffer into a stack, with stack_output() to push a new buffer on and pop_stack() to pop it off and return the output. (Feel free to suggest better names). Would that seem to be a comprehensive solution? Are there any other considerations before proceeding with that tweak?

Contributor

philsturgeon commented Oct 11, 2011

Sounds good to me but let's call in the big guns: @pkriete and @gaker. I know you guys are excited about having a little bit of HMVC in the core, can you give some feedback?

Contributor

dchill42 commented Oct 11, 2011

I have some tentative code worked up for the output stack feature:

  • Output initializes final_output to an array (stack) containing one empty string.
  • stack_push() pushes another empty string onto the stack and returns the stack level (e.g. - 2 after the first call), and will accept an optional parameter to initialize the new buffer to a non-empty string.
  • stack_level() returns the current stack depth as a convenience.
  • stack_pop() pops the end buffer off the stack (if there's more than one) and returns it, or just returns the bottom buffer contents if there is only one (i.e. - it prevents actually emptying the stack).
  • get_output(), set_output(), and append_output() all operate on the current buffer (top of stack).
  • _display() implodes the buffer stack, so all captured output (that hasn't been popped) is included in final output.

These names don't amuse me nearly as much as "get_forked" (snicker), but I think it's a sensible and completely backward-compatible solution which allows buffering sub-controller output (or any other desired subset) in an infinitely nestable manner (until you run out of memory).

I look forward to input from Greg and Pascal (and anyone else) before I commit this changeset.

Contributor

vlakoff commented on acb962e Jun 27, 2012

If the server is set to a non-English locale[1], ctype_alpha, ctype_alnum also match foreign accentuated characters like "é", "ö" and so on. It's roughly the same case as with \w in regexes.

For the sake of code portability and because the risk of breaking existing code is too high, I would suggest to undo these two.


[1] see [`setlocale`](http://www.php.net/manual/en/function.setlocale.php) (as the php.net doc may not suggest, the locale is not forcibly set at runtime, it may be set in system config)
Contributor

narfbg replied Jun 27, 2012

I'm aware of that.

narfbg and others added some commits Jun 27, 2012

Merge pull request #1319 from timw4mail/email
Make valid_email helper function more accurate
Respect display_errors when deciding if to display an error -- useful…
… for environments where you want to log errors, but not show them.
Merge pull request #1549 from chrisguiney/session_gc
Allow session garbace collection percentage to be configured.
Fix pdo_dblib and pdo_sqlsrv db_connect() and suppress warning messag…
…es for subdrivers that don't support certain option attributes
Clean up regexes in Security->xss_clean()
Removed some unneeded capturing groups (or made them non-capturing)
and some unneeded escape characters
Update Array Helper documentation
default value of the "default value" parameter has changed
in element() and elements() methods
Merge pull request #1561 from vlakoff/doc
Update Array Helper documentation
Merge pull request #1556 from valioz/develop
Added proxy to XML/RPC client
Merge pull request #1560 from vlakoff/regex
Clean up regexes in Security->xss_clean()

InFog and others added some commits Jul 29, 2012

Merge pull request #1670 from CaSoft/develop
Added documentation for mailto function
Merge pull request #1672 from alexbilbie/alexbilbie-email-clear
Automatically clear email parameters if send was successful
Merge pull request #1601 from rwillert/patch-1
Reconnect to PostgreSQL database if connection dropped.
Bug fix #1695 - change Nintendo mobile user agents to be more specific.
Signed-off-by: Eric Roberts <eric@cryode.com>
Contributor

narfbg commented on system/core/Input.php in d83e727 Aug 15, 2012

$proxies here is a non-existent variable. Should it be assigned from the proxy_ips config setting? @thecrypticace

Oh, brilliant… yes it should. Now fixed my repo.

Contributor

narfbg replied Aug 15, 2012

Can you try and tell if this patch works OK? https://gist.github.com/3359579
It should also support configuring proxy_ips directly as an array.

narfbg and others added some commits Aug 15, 2012

Remove change log entry for Bug fix #1695
Signed-off-by: Eric Roberts <eric@cryode.com>
Merge pull request #1698 from cryode/develop
Bug fix #1695 Update Nintendo mobile user agents.
Merge pull request #1714 from ollierattue/develop
Working mime types for xls and xlsx file extensions
Switching to Sphinx's built in JavaScript search
- disabled rST source copying, the contextual results for the search don't render, so they look goofy
- added .highlighted class to CSS for highlighting search terms
- replaced Google search form with native search form (and separated into its own theme file)
- added body class to #content div for JS highlighter to hook onto
Minor session test improvements
Signed-off-by: dchill42 <dchill42@gmail.com>
Extracted cookie database saves to shutdown and cleaned up code
Signed-off-by: dchill42 <dchill42@gmail.com>
Merge pull request #353 from dchill42/session
Switched Sessions to use Drivers
Adding native PHP sessions and keeping original-flavor CI cookie sessions
Contributor

dchill42 commented Oct 10, 2012

Incorporated into #1818

@dchill42 dchill42 closed this Oct 10, 2012

gangbo commented on 036b33b May 29, 2014

good

baypup pushed a commit to baypup/CodeIgniter that referenced this pull request Aug 20, 2015

Merge pull request #560 from appleboy/patch
update Chinese Traditional language.

Hi,

on your comment about custom_result_object() in DB_result, FWIW, if this statement:

elseif ( ! $this->result_id OR $this->num_rows === 0)

Is switched back to:

elseif ( $this->result_id === FALSE OR $this->num_rows === 0)

Then it makes a difference, because the cached result set has result_id set to NULL and it can be access it without querying the database again, by looping:

foreach ($query->result('Custom_Result_Object_Class') as $row)

With the current implementation, instead, if the database cache is on, it simply returns blank and the documentation does not mention this behavior, it seems something went wrong. From your previous comments about the DB_Cache, I have understood that there are some issues and that you want to deprecate this library, I would be interested to know more about the cases, to try to help or simply know when to avoid the database cache library.

In any case, thank you for your work.

Contributor

narfbg replied May 10, 2017

Oh, I see ... Didn't know you were doing this for caching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment