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 to register filters. #2819

Merged
merged 10 commits into from Jun 4, 2017
Merged

Allowing to register filters. #2819

merged 10 commits into from Jun 4, 2017

Conversation

felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Jun 4, 2017

Pull Request Checklist

  • I’ve read the guidelines for contributing.
  • I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
  • I tested my changes.

Description

Allows config plugins to register filters. Registers default filters with the names filters.<name>.

This implements the first part of the filters discussion in #2475 and is the first step of removing Python code from conf.py. This PR should not handle the shortcuts which allow to include Python code for filters (and global context) without creating plugins; that's for another PR.

@felixfontein felixfontein requested a review from Kwpolska Jun 4, 2017
# Ignore objects whose name starts with an underscore, or which are not callable
if filter_name.startswith('_'):
continue
if not hasattr(filter_definition, '__call__'):
Copy link
Member

@Kwpolska Kwpolska Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not callable(filter_definition):

Copy link
Contributor Author

@felixfontein felixfontein Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

# OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

"""Registers all default filters."""
Copy link
Member

@Kwpolska Kwpolska Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn’t need to be a plugin. Put this in nikola.py.

Copy link
Contributor Author

@felixfontein felixfontein Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't the idea in Nikola to move more stuff into plugins to make everything more modular? If this wouldn't need to be backwards compatible, I'd move the filter definitions into the plugin as well.

But if you prefer that, I can remove the plugin.

Copy link
Member

@Kwpolska Kwpolska Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, modularity is good, but we don’t need modularity for something there is no real need/reason for end users to disable. Post scanners are plugins, because you can have other ways of scanning posts. Setting up globals doesn’t need to be a plugin, because everyone needs those. Plus, reading (edit: more .plugin and .py files) can be costly (on Raspberry Pi SD cards, for example)

Copy link
Contributor Author

@felixfontein felixfontein Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, changed that.

@Kwpolska Kwpolska added this to the v7.8.8 milestone Jun 4, 2017
nikola/nikola.py Outdated
@@ -414,6 +414,7 @@ def __init__(self, **config):
self._template_system = None
self._THEMES = None
self._MESSAGES = None
self._filters = {}
Copy link
Member

@Kwpolska Kwpolska Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Underscore not needed.

Copy link
Contributor Author

@felixfontein felixfontein Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

nikola/nikola.py Outdated
if filter is not None:
actions[i] = filter

# Configure filters
Copy link
Member

@Kwpolska Kwpolska Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge the two loops.

Copy link
Contributor Author

@felixfontein felixfontein Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

nikola/nikola.py Outdated
one argument (the filename).
"""
prev_filter = self._filters.get(filter_name)
if prev_filter is not None:
Copy link
Member

@Kwpolska Kwpolska Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if filter_name in self.filters:

Copy link
Contributor Author

@felixfontein felixfontein Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

ralsina
ralsina approved these changes Jun 4, 2017
Copy link
Member

@ralsina ralsina left a comment

LGTM

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska Kwpolska merged commit 6fdaad3 into master Jun 4, 2017
@Kwpolska Kwpolska deleted the filter-registry branch Jun 4, 2017
@felixfontein
Copy link
Contributor Author

felixfontein commented Jun 4, 2017

Thanks for merging!

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

Successfully merging this pull request may close these issues.

None yet

3 participants