Fatal error on "View all committers" page because of tablesort_sql()

Thanks for the report!

Patch not really tested yet, but should work.

Almost working, thanks. One problem and one question:

A) Now that page isn't a fatal error, but it throws a PHP warning that $header is undefined on line 400:

  return theme('table', array('header' => $header, 'rows' => $rows));

versioncontrol_project_committers_page() is no longer responsible for $header. Honestly, this patch seems kinda weird with the default $header_with_order set to NULL and then versioncontrol_project_get_project_committers() becomes responsible for the header, etc. But, orderByHeader() doesn't make this particularly easy and clean for us.

B) I don't understand why $query is partially built up via separate calls to addExpression() and addField() and then for other things we chain the calls as we're saving the value to $result. Isn't The D7 Way(tm) to chain everything?

Thanks,
-Derek

A) The problem was really a bug, but yep, orderByHeader() make it really difficult to separate it, for now I think it will work.

B) Yep, that's on propose, not all methods that can be called from a SelectQueryInterface return a SelectQueryInterface, i.e. addExpression().

I'll try this patch before adding to upstream.

It is working :-)

Added to 7.x-1.x.

Yeah, that at least fixes the bug, but it's still weird. ;) We're now defining the default header twice. And it's strange that the API call for versioncontrol_project_get_project_committers() is intimately tied to tablesort like this.

Perhaps a better approach would be to pass in a sort field and order (more or less the D6 API), add those to the query with SelectQueryExtender::orderBy(), and use tablesort_get_sort() and tablesort_get_order() inside versioncontrol_project_committers_page() so that all the header nonsense can live in the function with the table.

However, that's still a bit wonky since the caller of versioncontrol_project_get_project_committers() has to know what fields/aliases are going into that query to know what the valid choices for the sort field should be. That's basically always a problem here. But, at least we could just document that in the PHPdoc and I think it'd be okay.

Another approach would be for the return value from versioncontrol_project_get_project_committers() to be a nested array with 'header' and 'rows' subarrays, so that it is solely responsible for crafting a valid $header with all the right field names/aliases. Callers can then use the header (even reorder or drop columns) if they need, or just inspect rows directly if not. We might still want to be able to pass in $sort and $order as above...

But, it's probably not worth making this elegant and beautiful at this point. ;) If the current code now works, we have more important things to do. So, I'm leaving this 'fixed'. Maybe we can work on a cleanup after we launch D7. ;)

Thanks!
-Derek

p.s. Giving this a better title for posterity.

Probably just needs deployment.

http://git7site.devdrupal.org/node/6018/committers looks okay