Backport to 6.x

Here is a backport for Drupal 6.

Great, I've been hoping someone would do this as it shouldnt be hard.

Could you maybe repost it as a patch against 7.x-1.x?

Here you are.

Looks like a great start, but not quite there yet.

First error I see is a notice about the type "container", which I'm assuming is a d7 thing:

  $form['settings'] = array(
    '#type' => 'container',
    '#states' => array(
      'invisible' => array(
        'input[name="xhprof_enabled"]' => array('checked' => FALSE),
      ),
    ),
  );  

Also, it appears that the function level pages are very broken still.

Fixed, so as the links to the function level pages. Some explanations in the next comment once the new patch is uploaded.

+++ b/XHProfRunsFile.inc
@@ -109,11 +109,13 @@ class XHProfRunsFile implements XHProfRunsInterface {
       foreach (glob("$dir/*.$source.*") as $file) {
         list($run, $source) = explode('.', basename($file));
+        $contents = $this->get_run($run, $source, $description);
         $runs[] = array(
           'run_id' => $run,
           'source' => $source,
           'basename' => htmlentities(basename($file)),
           'date' => date("Y-m-d H:i:s", filemtime($file)),
+          'path' => isset($contents['path'])?$contents['path']:'',
         );
       }

The path was not being loaded from the run file. That is why at admin/reports/xhprof there are no paths listed.

+++ b/xhprof.admin.inc
@@ -22,15 +21,7 @@ function xhprof_admin_settings() {
-  $form['settings'] = array(
-    '#type' => 'container',
-    '#states' => array(
-      'invisible' => array(
-        'input[name="xhprof_enabled"]' => array('checked' => FALSE),
-      ),
-    ),
-  );  
-  $form['settings']['xhprof_interval'] = array(
+  $form['xhprof_interval'] = array(

I simply removed the container and made the interval field a visible field.

+++ b/xhprof.inc
@@ -642,6 +642,7 @@ function xhprof_profiler_report($url_params, $rep_symbol, $sort, $run1, $run1_de
   $path_parts = explode('/', $_GET['q']);
   array_pop($path_parts);
+  array_pop($path_parts);
   $links[] = l("View Top Level $diff_text Report", implode('/', $path_parts));

Doing array_pop just once was leaving 'symbol' in the path, so links on function names were broken because of this.

+++ b/xhprof.module
@@ -102,12 +102,16 @@ function xhprof_is_enabled() {
-    if (arg(0) == 'admin' && variable_get('xhprof_disable_admin_paths', TRUE)) {
+    $path = isset($_GET['q'])?$_GET['q'] : '';

arg() is not available on hook_boot() at Drupal 6, so I am evaluating manually the querystring.

+++ b/xhprof.module
@@ -131,10 +135,10 @@ function xhprof_shutdown() {
-    drupal_register_shutdown_function('xhprof_shutdown_real');
+    register_shutdown_function('xhprof_shutdown_real');
 

There is no drupal_register_shutdown_function(). I have read that it is used mainly to catch possible exceptions. Should we catch them manually?

Oops, status is needs review.

Great, this one is working much better.

THe path thing isn't actually a bug, it's just awkward that it's only implemented by the mongo backend at the moment. I didn't want to add it to the file backend to keep compatibility with other xhprof readers.

Doing two array_pops feels dirty. We can probably come up with something better there.

+ $path = isset($_GET['q'])?$_GET['q'] : '';

Please add spaces per coding standards on ternaries.

As far as the shutdown is concerned, I forget what D6 does. The behavior is copied from devel, so lets just do whatever devel 6.x does.

Actually, I fixed the things I mentioned and committed it as the 6.x-1.x branch.

Please reopen if I missed anything.

THanks!

It would be nice to have at least a development version available at the project's page.