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

Allowing Nikola.path and Nikola.link to receive page number #2580

Closed
felixfontein opened this issue Dec 5, 2016 · 12 comments

Comments

@felixfontein
Copy link
Contributor

commented Dec 5, 2016

What do you think about extending the interface of Nikola.path and Nikola.link with an optional kw argument page (default value None)? This would allow to link directly to a certain page of an index, what currently is not possible except for the main index (the need for this was mentioned in #2077, and it would make calling generic_index_renderer a bit simpler since that would make implementing the page_link and page_path functions easier). Also, this could be used to directly link to a page of a post if #2257 is ever realized.

Any opinions?

@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

How about making it even more general by taking **kwargs and passing those to path handlers? That way, every path handler could have any fields it pleases, and we could support those in magic links.

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2016

Great idea!

One problem is implementing this, though, since existing path handlers don't expect this. How about adding a flag to register_path_handler which tells it that the registered handler supports arbitrary kwargs (its default value must be False, i.e. handler doesn't support it)?

@Kwpolska Kwpolska self-assigned this Dec 5, 2016
@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

There is no problem. If you call the function with name, lang, **kwargs, and you don’t provide any arguments in the original **kwargs for those old path handlers, they will get 2 arguments.

>>> def foo(a, b):
...     print(a, b)
...
>>> def caller(a, b, **kwargs):
...                # ↑↑ optional, but provides friendly API
...             # ↓↓ mandatory
...     foo(a, b, **kwargs)
...
>>> caller(1, 2)
1 2
>>> caller(1, 2, bar=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 2, in caller
TypeError: foo() got an unexpected keyword argument 'bar'
@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

I’ll handle this when I have a moment. But first, I’ve got an issue I need to hunt down.

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2016

Ok.

How about the existing path handlers: should they accept **kwargs by default and ignore its contents (if they don't know it explicitly)? Or should they not accept it and let compilation die if a wrong argument is encountered?

@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

They can be left as-is if they don’t want to use any new keyword arguments yet. In fact, nobody needs to accept **kwargs, they could only do page=None and completely ignore the underlying **kwargs thing.

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2016

Accepting **kwarg might make developing backwards-compatible plugins and themes easier in the future, so that plugins and themes could supply arguments which the current Nikola installation doesn't know about yet. I'm not completely convinced by this argument, though...

@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

How would those plugins work, monkeypatching stuff? The caller functions (Nikola.path, Nikola.link) will take **kwargs and then pass those as keyword arguments to the path handler. The path handler may use **kwargs on its own, but that’s generally unnecessary, because in most cases it couldn’t do anything smarter.

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2016

With backwards compatible I don't mean the currently released version of Nikola, but future ones. They would then accept arguments they don't know about but versions more in the future would accept.

@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

That would involve ignoring whatever garbage **kwargs gets, and that sounds bad to me.

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2016

I can see both advantages and disadvantages for that approach. That's why I was asking, and your "sounds bad to me" answers my question :)

Kwpolska added a commit that referenced this issue Dec 6, 2016
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2016

This is now implemented in #2585, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.