There is no way to initialize a pager without running an SQL query

Patch looks good, let's see what the test bot says.

I think the patch is ok; testbot was glitchy. I'm seeing fails locally, but I don't think they are related to this patch.

Hmmm. Perhaps rename to pager_default_initialize since we deliberately left the door open in D7 to alternate pager classes.

Patch applies and works. Might be nice to add some docblock comments about how to add a custom pager.

Yes, let's get some how-to PHPDoc in here before this is committed.

Also, is there any particular reason we don't just make this part of the constructor of PagerDefault? Then PagerSuperAwesome could just do a similar thing in its constructor.

A few minor quibbles about comments, and one about code flow:

+++ includes/pager.inc
@@ -171,6 +158,39 @@ class PagerDefault extends SelectQueryExtender {
+ *  The total number of items found.

"found" is kind of jarring here, since we're talking about items in general, not search results.

+++ includes/pager.inc
@@ -171,6 +158,39 @@ class PagerDefault extends SelectQueryExtender {
+ *  The number of items you will display per page.

Who is "you"? The code, the server or the developer? ;)

+++ includes/pager.inc
@@ -171,6 +158,39 @@ class PagerDefault extends SelectQueryExtender {
+  $page = isset($_GET['page']) ? $_GET['page'] : '';
+
+  // Convert comma-separated $page to an array, used by other functions.
+  $pager_page_array = explode(',', $page);

It's no real performance overhead, but do you really need to re-populate the global variable on every call? Just do it once or something...

This review is powered by Dreditor.

Well, PagerDefault is a class because it extends PDO. If we conclude that all pagers should be classes, then what would "PagerWithoutDatabase" extend? It can't exactly extend the SelectQueryExtender like PagerDefault does. (Nor can we make both extend a common Pager class, since that would involve PagerDefault extending two classes.)

In fact, the "PagerWithoutDatabase" pager would be completely unrelated to the database part, so I'm not sure why it should be a class.

#22: pager-init-667112-22.patch queued for re-testing.

This looks RTBC to me...thanks to webchick for prodding on the documentation, it was very much needed. :)

Oh my god what a useful patch, half of the very simplistic Views backends I wrote are repeatitions of this patch, yes, yes please do!

+++ includes/pager.inc
@@ -73,22 +72,10 @@ class PagerDefault extends SelectQueryExtender {
+    $this->range($current_page * $this->limit, $this->limit);
+    ¶
     // Now that we've added our pager-based range instructions, run the query normally.

trailing whitespace

Powered by Dreditor.

In addition to the whitespace issues, the attached version:

• Adds even more exciting PHPDoc comments (because pagers are confusing and need all the docs they can get), and updates the example code a bit for D7 changes that occurred in the interim.
• Clarifies the documentation for the return value of pager_find_page().
• No longer uses a global variable inside pager_find_page() - turns out we didn't need it there, but only in the other function, and the fewer global variables the better.

Other than that, I've reviewed and tested this patch pretty carefully, and it's great. The actual code changes are pretty simple - mostly just moving existing code out into reusable functions - but the benefits are huge. In addition to Solr, the field API is another application in D7 where this is useful. Example: We are trying to build a D7 media gallery and want to page through the results of a multivalued media field on a node. We can't use the PagerDefault extender for that since field API queries are not tagged (and even if they were tagged, it wouldn't be a good idea, since the field API is supposed to be storage-agnostic).

This patch makes things work perfectly for that purpose, so hopefully we can get it in.

This patch can't be committed. Really. What would happen to our cushy Drupal consultants jobs if we would make pager easy to use? Come on. First you remove the seven arguments of l() now it's pager globals? At this pace it won't take another 4-6 releases before any Dick or Harry can write Drupal code. We can't have that.

Committed to CVS HEAD. Thanks.