Create more nuanced code to turn XHGui on

[perftools]

Create more nuanced code to turn XHGui on

[markstory]

This is somewhat related to #17 which also requested a way to manually enable profiling.

One option is the inclusion of a simple 'rules' system that could iterate a set of criteria to see if the request should be profiled. It could either be built with a series of closures, or a more declarative system.

[preinheimer]

I think this is a great feature to leave to leave to a new contributor, it doesn't require a lot of knowledge of the code base, nor a particular understanding of the frameworks but is quite meaningful.

[markstory]

I'm going to poke around on this, I have some ideas on how this could be built. I'd also like to move the settings file into a settings.default that gets copied into place by the installer.

[anho]

-1 on the idea of a file being copied by the insteller. I'd go with a default file that is shipped with the application and the ability to override settings in a second file. So you get something like config.global.php & config.local.php.

[markstory]

@anho That is effectively the same thing, but with one fewer file being loaded on each request.

[anho]

It's not when you are upgrading existing installations and xhgui added further settings to its config. In that case the users have to edit their own config file accordingly to be able to run the app properly. If you provide an actual default config and let the user override only single settings as he wants, you create less friction.

Or am I missing the details of the implementation?

[markstory]

Hmm good point about making upgrading easier. I was thinking of having the following in the configuration file:

return array(
  // Other config
  'profiler.enable' => array(
    // Profile 1 in 100 requests.
    function () { return rand(0, 100) !== 42; },
    function () {
      // User land code.
    }
  )
);

This would let people include any environment based conditions they needed. It also lets xhgui come with a reasonable default of 1 in 100 requests. If any of the callable functions returns true, then the profiler would be enabled.

Does this seem like a reasonable approach overall?

[anho]

Do you see any use case where the user wants to have more than one callable determining the state of the profiler? I would say: YAGNI.
If I would have a more complex code to enable the profiler I'd just use a class hiding the details. So one callable would be enough.

[markstory]

Fair point.