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

Added static_comments plugin. #201

Merged
merged 10 commits into from Jan 8, 2017
Merged

Added static_comments plugin. #201

merged 10 commits into from Jan 8, 2017

Conversation

@felixfontein
Copy link
Contributor

felixfontein commented Jan 8, 2017

A plugin which allows to add static comments which don't require JavaScript. Needs theme support to actually show the comments.

@felixfontein
Copy link
Contributor Author

felixfontein commented Jan 8, 2017

There seems to be something wrong with the tests @Kwpolska (related to the helloworld plugin)'; I guess this is a result of moving the v6 plugins to the v7 folder.


the content spans the rest of the file.

Most header fields are optional. `compiler` must specify a page compiler which allows to compile a content given as a string to a string; these are currently the restructured text compiler (`rest`), the `ipynb` compiler, and the [WordPress](https://plugins.getnikola.com/#wordpress_compiler) (`wordpress`) compiler. Comments can form a hierarchy; `parent_id` must be the comment ID of the parent comment, or left away if there's no parent.

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jan 8, 2017

Member

I’d prefer to add html and markdown. Also, remove ipynb because that’s going too far.

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jan 8, 2017

Author Contributor

Well, ipynb is one of the two default plugins providing such a function. Markdown isn't (yet). With html, do you mean "just treat the content as HTML", or do you want the content to be processed by the html page compiler plugin (which also processes shortcodes)?

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jan 8, 2017

Member

Comments are meant to be small and simple, they do not need shortcodes, notebooks, or other magic. Just make it basic HTML passthrough.

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jan 8, 2017

Author Contributor

Well, it should be up to the blog admin to decide which page compiler he/she whats to use for which comments. If for whatever reason they want to use ipynb, they can do so because that plugin supports compiling from a string to a string.

I'll add passthrough support for html.

I can also add such a function to the markdown plugin in Nikola core to enable using markdown as a comment compiler.

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jan 8, 2017

Member

Sure, go ahead with markdown.

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jan 8, 2017

Author Contributor

Hmm, I just noticed I didn't look at the ipynb plugin close enough. The compile_string function wants a file name, not the content.

_header_regex = re.compile('^\.\. (.*?): (.*)')

def _compile_content(self, compiler_name, content, filename):
"""Compile comment content with """

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jan 8, 2017

Member

with what?

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jan 8, 2017

Author Contributor

Fixed.

return compiler.compile_string(content)
else:
try:
return compiler.compile_to_string(content) # this is a non-standard function! must not be available with any page compiler!

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jan 8, 2017

Member

May not* and who provides compile_to_string?

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jan 8, 2017

Author Contributor

Fixed.

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jan 8, 2017

Member

Who provides compile_to_string and why is that distinct from compile_string?

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jan 8, 2017

Author Contributor

The WordPress compiler provides compile_to_string, while rest and ipynb provide compile_string (though rest doesn't simply return a string, but a tuple).


{% macro add_static_comments(static_comment_list, lang) %}
{%if static_comment_list|length == 0 %}
<div class="no-comments">{{ messages("No comments.", lang) }}</div>

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jan 8, 2017

Member

How would that work? Does this plugin require manual messages customization?

This comment has been minimized.

Copy link
@felixfontein

felixfontein Jan 8, 2017

Author Contributor

Yes, it does. I'll mention that in the instructions.

…tring function support.
@Kwpolska

This comment has been minimized.

Copy link
Member

Kwpolska commented on v7/static_comments/README.md in 339ae5e Jan 8, 2017

you can say v7.8.2 here

@Kwpolska

This comment has been minimized.

Copy link
Member

Kwpolska commented on 339ae5e Jan 8, 2017

👎, the API should be compile_string to match the official, built-in reST plugin.

This comment has been minimized.

Copy link
Contributor Author

felixfontein replied Jan 8, 2017

It matches it as closely as it can. The second return argument makes no sense for MarkDown.

felixfontein and others added 4 commits Jan 8, 2017
@felixfontein
Copy link
Contributor Author

felixfontein commented Jan 8, 2017

LGTM. My blogs still compile and produce the same output.

Copy link
Member

Kwpolska left a comment

👍 but please don’t merge this yet so I don’t have to fight with merge conflicts.

@felixfontein
Copy link
Contributor Author

felixfontein commented Jan 8, 2017

Sure, fine for me. Merge whenever you are done.

@Kwpolska Kwpolska merged commit 8586402 into master Jan 8, 2017
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
@Kwpolska Kwpolska deleted the static_comments branch Jan 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.