Skip to content
Permalink
Browse files
Add CompilerExtension class (cc @felixfontein)
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
  • Loading branch information
Kwpolska committed Jul 4, 2015
1 parent d3fd670 commit c65be6e24a5daaa3d1b7526ae9dcce7daf017f64
@@ -60,6 +60,7 @@
Command,
LateTask,
PageCompiler,
CompilerExtension,
RestExtension,
MarkdownExtension,
Task,
@@ -668,6 +669,7 @@ def __init__(self, **config):
"TemplateSystem": TemplateSystem,
"PageCompiler": PageCompiler,
"TaskMultiplier": TaskMultiplier,
"CompilerExtension": CompilerExtension,
"RestExtension": RestExtension,
"MarkdownExtension": MarkdownExtension,

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jul 4, 2015

Contributor

Why not remove RestExtension and MarkdownExtension from this list, and handle them like any other CompilerExtension should be? And add a helper function which takes care of DISABLED_PLUGINS?
I'll start a branch for that.

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jul 4, 2015

Author Member

We are not breaking backwards compatibility here. You can certainly write a helper function, but leave the two classes alone.

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jul 4, 2015

Contributor

Why is this breaking backwards compatibility?

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jul 4, 2015

Contributor

My implementation: https://github.com/getnikola/nikola/compare/unifying-compiler-extensions?expand=1
I'm not sure how to actually use CompilerExtensions without changing the Rest and markdown compiler, except if activating a pluing multiple times is no problem for yapsy (haven't tested that). If I'm trying to use yapsy's is_activated (see http://yapsy.sourceforge.net/PluginInfo.html), I get an exception when the GistExtension plugin is processed:

Traceback (most recent call last):
  File "/usr/bin/nikola", line 9, in <module>
    load_entry_point('Nikola==7.5.1', 'console_scripts', 'nikola')()
  File "~/nikola/nikola/__main__.py", line 158, in main
    site = Nikola(**config)
  File "~/nikola/nikola/nikola.py", line 732, in __init__
    plugin_info.plugin_object.set_site(self)
  File "~/nikola/nikola/plugins/compile/rest/__init__.py", line 131, in set_site
    for plugin_info in site.activate_compiler_extensions('rest'):
  File "~/nikola/nikola/nikola.py", line 847, in activate_compiler_extensions
    print(plugin_info.is_activated)
  File "/usr/lib/python3.4/site-packages/yapsy/PluginInfo.py", line 193, in _getIsActivated
    return self.plugin_object.is_activated
AttributeError: 'GistExtension' object has no attribute 'is_activated'

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jul 4, 2015

Author Member

In my solution, the new code would have to query Nikola/Compiler from the .plugin file, a field which I just added, to find out which extension plugins to activate for this compiler. Compiler plugins that do not have that field would crash Nikola if you did not catch the exception. Turns out plugin activation does pretty much nothing, so we could just talk to plugin_object anyway, which is guaranteed to have a compiler_name attribute.

I would rather put the function in CompilerExtension.activate_extensions (and not the site object, to make things cleaner) to be used by everyone.

is_activated should be handled by yapsy (in IPlugin.__init__), no idea why that does not happen here.

Please move activate_extensions to CompilerExtension (as there is no reason to put it in nikola.py) and create a pull request.

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jul 4, 2015

Author Member

PS. I’m leaving the Nikola/Compiler field in, it could come in useful if we ever implement some category system on the plugins site.

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jul 4, 2015

Contributor

Is yapsy offering a nice way to get to the contents of the parsed .plugin file? Then reading Nikola/Compiler would be easy.
So how do you want that activate_extensions of CompilerExtension be implemented? As a static method, called with site or site.compiler_extensions? Anyway, that doesn't really look clean to me. I'd rather put it into PageCompiler as a non-static method, because there it fits much better.

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jul 5, 2015

Author Member

I actually meant to say PageCompiler, sorry. It does not make any sense in CompilerExtension, in fact.

To read Nikola/Compiler, you can use plugin_info.details.get('Nikoala', 'Compiler'), but it’s better if you didn’t — we can just talk to plugin_info.plugin_object.compiler_name without much trouble, which will spare us has_section and has_option checks and workarounds for old reST and Markdown extensions.

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jul 5, 2015

Contributor

Ok, then we both agree on that :)

I also agree that we shouldn't use Nikola/Compiler yet. How about emitting a warning if Nikola/Compiler is not set? Or should we wait for v8 to do that (since right now, most page compilers in the open won't have it and it might confuse users)?

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jul 5, 2015

Author Member

Let’s not even bother enforcing it, it’s not really useful for Nikola itself and will be used only for the plugin page.

"SignalHandler": SignalHandler,
@@ -284,12 +284,34 @@ def split_metadata(self, data):
return split_result[0], split_result[-1]


class RestExtension(BasePlugin):
class CompilerExtension(BasePlugin):

"""An extension for a Nikola compiler.
If you intend to implement those in your own compiler, you can:
(a) create a new plugin class for them; or
(b) use this class and filter them yourself.
If you choose (b), you should the compiler name to the .plugin
file in the Nikola/Compiler section and filter all plugins of
this category, getting the compiler name with:
p.details.get('Nikola', 'Compiler')
Note that not all compiler plugins have this option and you might
need to catch configparser.NoOptionError exceptions.
"""

name = "dummy_compiler_extension"
compiler_name = "dummy_compiler"



class RestExtension(CompilerExtension):
name = "dummy_rest_extension"
compiler_name = "rest"


class MarkdownExtension(BasePlugin):
class MarkdownExtension(CompilerExtension):
name = "dummy_markdown_extension"
compiler_name = "markdown"


class SignalHandler(BasePlugin):
@@ -2,6 +2,9 @@
Name = mdx_gist
Module = mdx_gist

[Nikola]
Compiler = markdown

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = mdx_nikola
Module = mdx_nikola

[Nikola]
Compiler = markdown

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = mdx_podcast
Module = mdx_podcast

[Nikola]
Compiler = markdown

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_chart
Module = chart

[Nikola]
Compiler = rest

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_doc
Module = doc

[Nikola]
Compiler = rest

[Documentation]
Author = Manuel Kaufmann
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_gist
Module = gist

[Nikola]
Compiler = rest

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_listing
Module = listing

[Nikola]
Compiler = rest

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_media
Module = media

[Nikola]
Compiler = rest

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_post_list
Module = post_list

[Nikola]
Compiler = rest

[Documentation]
Author = Udo Spallek
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_slides
Module = slides

[Nikola]
Compiler = rest

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_soundcloud
Module = soundcloud

[Nikola]
Compiler = rest

[Documentation]
Author = Roberto Alsina
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_thumbnail
Module = thumbnail

[Nikola]
Compiler = rest

[Documentation]
Author = Pelle Nilsson
Version = 0.1
@@ -2,6 +2,9 @@
Name = rest_vimeo
Module = vimeo

[Nikola]
Compiler = rest

[Documentation]
Description = Vimeo directive

@@ -2,6 +2,9 @@
Name = rest_youtube
Module = youtube

[Nikola]
Compiler = rest

[Documentation]
Version = 0.1
Description = Youtube directive

0 comments on commit c65be6e

Please sign in to comment.