Static comments

[felixfontein]

Hi,

I saw that there once was a discussion on static comments (#719). At that time, the idea was more or less dropped, as far as I understand. I'd like to revive it to some extend :) Namely, I want to convert some of my WordPress blogs including all comments. I don't want to use any JavaScript or any 3rd party system which requires access to another server, and I also don't want to just drop the comments. I'm not sure what to do about commenting in the future, but for now (also due to a very low volume of comments), I'd be perfectly happy with static comments. The problem is that it looks like I need a bit of support by Nikola to do this. In particular, I need that for every created post object, one function is called. From what I understand, there's no way to do that with the current plugin types, so I need a new one for precisely this.

Adding that is quite simple (also adding support for static comments in galleries, this requires essentially one call per gallery). The question is how far should this go, i.e. whether there should be more support for static comments or not. The bare minimum is a plugin category which is called once per Post object (for both posts and stories) and once per gallery (with the chance to return something which is added to the gallery's context). With that foundation, doing everything else in a plugin (or multiple plugins) and themes is definitely possible. A more deluxe solution would be to add basic support to Nikola, in form of a Comment class collecting some information on comments. Then it would be much easier to support generic static comments in a theme without knowing which static comments plugin the user might use (maybe also none at all). Of course, one question is: how many such plugins will there be after all? Probably not many. Maybe only one. So I'm really not sure if it's worth that trouble to add such things. Maybe it's better to only implement the minimal foundation and leave everything else to plugins?

What do you think? Do you want such a feature at all? And if you want it, do you want more a minimalist, or more a deluxe version?

Thanks and all the best, Felix

[ralsina]

This is exactly why I wanted hooks :-)

When you create a post, you will get a blinker signal called new_post or new_story IIRC

To make that trigger your own code, you can create a new plugin that connects a callable to that signal.

If you need to have other things trigger your code, then we probably need to add more hooks in other places and give you more signals to connect to.

[felixfontein]

Ah, that sounds good as well. Then everything can be implemented in a single plugin without any loss of comfort and without modifying Nikola, I assume. I only found a scanned signal (doing something close to notifying about posts and stories), sending a message when all posts and stories are scanned. But that should be completely fine for me, I can then iterate through all posts and stories by myself :)

[ralsina]

@felixfontein it's there in https://github.com/getnikola/nikola/blob/master/nikola/plugins/command/new_post.py#L344 :-)

Yes, the idea was to be able to write plugins that react to events onbuilds or whatever. Congrats this is the 1st time someone will use it, I think :-D

[Kwpolska]

You cannot stand 3rd-party services. But, could you at least use isso? It’s JavaScript, but this should not matter to you. You host your own isso instance on your server.

About implementing static comments: this is always a big burden on the site author — in other words, you. You get a fancy e-mail for every comment — and then you must put it somewhere where your generator will see it. And then rebuild the entire site. That’s a lot of work a dynamic thing could easily do for you.

Here’s all the work you’d need to do if you still want to create such a system:

You’d basically need to write a new plugin specifically for that and attach some templates to it. Your plugin also must tell the posts tasks that a rebuild is needed — and for that, you will need to modify Nikola — at least add a way to add dependencies from plugins…

[felixfontein]

Ah, but that is only run on creating that post, and not once per Nikola run per post (that's what I actually meant -- now I realize it wasn't clear that I meant every run of Nikola :) ).

And cool, I love trying out new things ;-)

[Kwpolska]

@ralsina Or is it? What use would this signal have for the static comments thing?

[felixfontein]

@Kwpolska that signal isn't of much use, probably, but scanned will do. It's not called per post and story, but after Nikola is done scanning all posts and stories.

[felixfontein]

@Kwpolska regarding isso: I'd prefer something which works without Javascript. And isso won't be flexible enough to run the comment through page compiler plugins (that's what I'm planning to do with my own static comments plugin, to have support for certain other things).

Maybe I'll try a hybrid in the future, something which displays new comments via JavaScript and renders old comments by a cronjob starting Nikola every night or so. But for the moment, I'd really like to stick to a purely static solution.

[Kwpolska]

@felixfontein Acknowledged. Still, you would need to modify the core, and allow at least dependency injection from your plugin. You could accomplish this by some extra_deps list/dict in the post object that the posts task then injects (as we do with reST deps)

[felixfontein]

Yes, the dependency problem is the hardest part. Currently I stick to nikola build -a, but that somewhat sucks ;-) I'll take a closer look at that when I'm trying to do as much as possible from inside a plugin.

[ralsina]

I think it may be possible to add dependencies to posts from plugins via monkey-patching RenderPosts.gen_tasks at least.

[ralsina]

@felixfontein @Kwpolska One way to do dependency injection is to write a TaskMultiplier plugin.

Look at https://github.com/getnikola/nikola/blob/master/nikola/plugins/task/gzip.py for an example. That plugin yields two tasks for each file worth gzipping.

A plugin of this type could just as easily modify the task's file_dep list and add whatever to it.

[felixfontein]

@ralsina Using TaskMultiplier unfortunately doesn't work, because the task dict is already converted to a doit Task object by doit.loader.generate_tasks before the TaskMultiplier object get's its hand on it.

@ralsina @Kwpolska What do you all think about adding blinker hooks to Nikola.generic_page_renderer, Nikola.generic_post_list_renderer (and generic_rss_renderer while we're at it) to allow adding more dependencies to the generated tasks? I.e., send the task object to be generated to the hook and let it add/modify the task object.

[felixfontein]

Ah, adding a hook to generic_rss_renderer makes no sense as it is not spitting out tasks. So just generic_page_renderer and generic_post_list_renderer, then :)

[ralsina]

@felixfontein passing the task to hooks may work since dicts are mutable but it sounds messy. You can modify Task objects, right? Looking at doit source code, it's just assigning to Task.file_dep and yielding the same object.

[felixfontein]

I could do that, but I can't get my hand on the generated Task object. (Not without changing __main__.py, though.)

I also need the task object to look in its action field, find all calls to Nikola.render_template, extract their context, and get the list of posts from there. From that list (only one post for rendering single pages), I generate a hash of all comments associated to all these posts. This I essentially add to to the uptodate field -- unfortunately, I have to do an ugly hack there since the object is later modified under the assumption that uptodate consists precisely of one config_changed object. The latter fact took me more than one hour to find that out; for example the pages plugin does the following:

                    task['uptodate'] = [config_changed({
                        1: task['uptodate'][0].config,
                        2: kw})]

I guess the reason for that is because doit doesn't like more than one config_changed object per uptodate list.

At the moment I don't see any cleaner way to do this (except using file_deps, but that would have to happen through hooks as well). But that would require comments to come from files (and not from any other "magic" source).

[felixfontein]

Maybe the cleanest solution would be to add a field dep_values (of type dict) to the Post class to which plugins can simply add something, and which will be somehow inserted into deps_dict resp. deps_context in generic_page_renderer resp. generic_post_list_renderer.

[ralsina]

@felixfontein OK, I am rather lost at this point, so I'll take your word for it.

Just some things that come to mind:

You can tell what tasks are about building post pages by looking at the task basename, right? I am sure we can add extra metadata to the task somehow if needed too.

[felixfontein]

Probably, yes, but how does it help me to set dependencies?

[ralsina]

If you want to inject dependencies in the page of each post so it's rebuilt when there are new comments, all you need is to catch each task that builds a post's page and add dependencies on its comments. Or am I missing something else?

To do that, you can use a TaskMultiplier, use basename and whatever to figure out if it's a task that builds a post's page, and then inject extra file_deps so that it's rebuilt as needed.

[felixfontein]

I want to inject dependencies into each page for a post, and into each page list containing that post. It's easy to catch everything generated via Nikola.generic_xxx_renderer by looking at the task's actions, but the basename could be anything.

Also, I'd like to avoid file dependencies if possible. I prefer dependencies based on changes of hash values (which the static comments plugin can generate).

The main problem doesn't change, though. At the point the TaskMultiplier gets the task it cannot modify it anymore since it's already generated, and it has no access to the generated Task object. So there's no way to modify either file_deps nor uptodate for that task.

[felixfontein]

It would be possible, though, if Nikola.gen_tasks would be modified as follows:

        for pluginInfo in self.plugin_manager.getPluginsOfCategory(plugin_category):
            for task in flatten(pluginInfo.plugin_object.gen_tasks()):
                assert 'basename' in task
                task = self.clean_task_paths(task)
                tasks = [task]
                for multi in self.plugin_manager.getPluginsOfCategory("TaskMultiplier"):
                    flag = False
                    for task in multi.plugin_object.process(task, name):
                        flag = True
                        tasks.append(task)
                    if flag:
                        task_dep.append('{0}_{1}'.format(name, multi.plugin_object.name))
                for task in tasks:
                    yield task
            if pluginInfo.plugin_object.is_default:
                task_dep.append(pluginInfo.plugin_object.name)

i.e. it first collects all tasks coming from a task plugin and the tasks the TaskMultipliers generate out of it in a list, and then emitting them in a loop. This would allow the TaskMultiplier objects to modify the original task (by appending something to task.file_deps or task.uptodate or something way more stupid).

[schettino72]

On Thu, Dec 4, 2014 at 8:16 AM, felixfontein notifications@github.com wrote:

I could do that, but I can't get my hand on the generated Task object. (Not without changing main.py, though.)

I also need the task object to look in its action field, find all calls to Nikola.render_template, extract their context, and get the list of posts from there. From that list (only one post for rendering single pages), I generate a hash of all comments associated to all these posts. This I essentially add to to the uptodate field -- unfortunately, I have to do an ugly hack there since the object is later modified under the assumption that uptodate consists precisely of one config_changed`object. The latter fact took me more than one hour to find that out; for example thepages`` plugin does the following:

                task['uptodate'] = [config_changed({
                    1: task['uptodate'][0].config,
                    2: kw})]

I guess the reason for that is because doit doesn't like more than one config_changed object per uptodate list.

Thats true. If config_changed() is used more than once the values will be overwritten... Thats a problem of the config_changed() implementation and I guess not hard to find a work around.

cheers,

[felixfontein]

Yes, it's quite simple to do that. The idea would be to create an own config_changed class which allows to specify the name of the variable used to store the config's hash. Actually, in Nikola.utils there's already a class deriving from doit's config_changed (it uses a different serialization), so a natural choice would be to extend that class to allow setting the variable name, and then using a name based on the plugin's name (or so) and avoid such rather hackish code.

[felixfontein]

Ok, to get rid of the restrictions of config_changed and the hacks resulting of the old behavior I now started a pull request (#1526).

About the rest: what do you guys think about extending the Post class so that you can add dependencies (for dep_files or uptodate) for

• either the post itself (which includes all pages and indices and ... using that post);
• or only for everything using that post except the initial post rendering (this is important for comments, since when adding/removing comments, everywhere the Post object is used needs to be re-run, but the original compilation doesn't need to be done).

The core plugins and generic_post_list_renderer and generic_page_renderer need to be modified to use this information resp. to use this functions to store information.

Comments?

[Kwpolska]

Makes sense. Bonus points for integrating the .dep file logic with this.

[felixfontein]

I'd like to move the .dep file logic into the page compiler. For example, a function of the page compiler is called directly after creation of the Post objects, whose sole job is to include the contents of .dep files (or something similar, or just nothing, depending on the page compiler).

I'll probably move the .dep handling into the default implementation in plugin_categories.py, so that other page compilers people wrote who depend on the .dep file logic don't break. Page compilers who don't need it can overwrite the default implementation to do nothing.

[felixfontein]

Ok, development started. Let's continue discussion of concrete implementation stuff in #1529, and use this thread only for more philosophical stuff (or close it).

[felixfontein]

Since both PRs which resulted from this thread are merged, I guess it's time to close it. Thanks for your feedback and hints!