Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define URL generator when instantiating Page #2

Closed
rbu opened this issue Jul 3, 2013 · 5 comments
Closed

Define URL generator when instantiating Page #2

rbu opened this issue Jul 3, 2013 · 5 comments

Comments

@rbu
Copy link

rbu commented Jul 3, 2013

Defining the page_url from the template with the $page replacement protocol does not seem like the right place to us. It would be much more convenient, if we could alternatively provide a URL generation function during instantiation.

From an API side, this could be used as:

def page_url_maker(page_number):
     return "http://example.com?page=%s" % page_number 
Page(items, number, page_url_maker=page_url_maker)
...

# template
{{ page.pager() | safe }}

In Pyramid, a simple page maker could be

    def make_page_url(page_number):
        return request.current_route_url(_query=dict(page=page_number))

We are currently doing this in a Page subclass, but getting it upstream would be preferable. I can provide a pull request, but wanted to make sure this has a chance of getting accepted first.

@Signum
Copy link
Contributor

Signum commented Jul 3, 2013

Hi Robert,

yes, I was considering such a URL generation callback. My first priority
was to make paginate independent from Pylons or Pyramid by using as few
external dependencies as possible. For many people it might be
sufficient to just use a page parameter but I fully understand that you
would like to have more control.

So if you could provide a proper patch I'll merge it. Thanks.

Kindly
…Christoph

rbu added a commit to rbu/paginate that referenced this issue Jul 3, 2013
This makes it possible to specify a function to generate the URL for a
page during instantiation instead of from the template.

When used in a Pyramid project, this could serve as an example:

    def url_maker(page_number):
        return request.current_route_url(_query=dict(page=page_number))

Fixes Pylons#2
@rbu
Copy link
Author

rbu commented Jul 3, 2013

I'm all in favor of not sticking to one framework and keeping it free from the magic you removed in 99b590e.

Pushed a patch to my fork. Thanks for offering to merge!

@rbu
Copy link
Author

rbu commented Jul 3, 2013

Relating to my commit message. Until Pyramid solves Pylons/pyramid#1040, you'll have to write this as

def url_maker(page_number):
    query = request.GET
    query['page'] = str(page_number)
    return request.current_route_url(_query=query)

@rbu
Copy link
Author

rbu commented Jul 29, 2013

We're currently shipping a fork of paginate in our product including this commit. Generally, I'd prefer not to derive from upstream too much. Can you give some general feedback on the patch's acceptability? What are the chances of seeing a merge / release?

@Signum
Copy link
Contributor

Signum commented Jul 29, 2013

Sorry, I missed your commit. Thanks for the patch - I have merged it. Please next time consider sending a pull request.

@Signum Signum closed this as completed Jul 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants