Merge branch 'master' of ssh://git.ikiwiki.info/srv/git/ikiwiki.info

master
Joey Hess 2008-11-28 08:53:35 -08:00
commit 93a43084d0
5 changed files with 201 additions and 172 deletions

View File

@ -5,172 +5,11 @@ This plugin adds "blog-style" comments. The intention is that on a non-wiki site
(like a blog) you can lock all pages for admin-only access, then allow otherwise
unprivileged (or perhaps even anonymous) users to comment on posts.
Comments are saved as internal pages, so they can never be edited through the CGI,
only by direct committers. Currently, comments are always in [[ikiwiki/markdown]].
> So, why do it this way, instead of using regular wiki pages in a
> namespace, such as `$page/comments/*`? Then you could use [[plugins/lockedit]] to
> limit editing of comments in more powerful ways. --[[Joey]]
>> Er... I suppose so. I'd assumed that these pages ought to only exist as inlines
>> rather than as individual pages (same reasoning as aggregated posts), though.
>>
>> lockedit is actually somewhat insufficient, since `check_canedit()`
>> doesn't distinguish between creation and editing; I'd have to continue to use
>> some sort of odd hack to allow creation but not editing.
>>
>> I also can't think of any circumstance where you'd want a user other than
>> admins (~= git committers) and possibly the commenter (who we can't check for
>> at the moment anyway, I don't think?) to be able to edit comments - I think
>> user expectations for something that looks like ordinary blog comments are
>> likely to include "others can't put words into my mouth".
>>
>> My other objection to using a namespace is that I'm not particularly happy about
>> plugins consuming arbitrary pieces of the wiki namespace - /discussion is bad
>> enough already. Indeed, this very page would accidentally get matched by rules
>> aiming to control comment-posting... :-) --[[smcv]]
>> Thinking about it, perhaps one way to address this would be to have the suffix
>> (e.g. whether commenting on Sandbox creates sandbox/comment1 or sandbox/c1 or
>> what) be configurable by the wiki admin, in the same way that recentchanges has
>> recentchangespage => 'recentchanges'? I'd like to see fewer hard-coded page
>> names in general, really - it seems odd to me that shortcuts and smileys
>> hard-code the name of the page to look at. Perhaps I could add
>> discussionpage => 'discussion' too? --[[smcv]]
>> (I've now implemented this in my branch. --[[smcv]])
>> The best reason to keep the pages internal seems to me to be that you
>> don't want the overhead of every comment spawning its own wiki page.
>> The worst problem with it though is that you have to assume the pages
>> are mdwn (or `default_pageext`) and not support other formats. --[[Joey]]
>> Well, you could always have `comment1._mdwn`, `comment2._creole` etc. and
>> alter the htmlize logic so that the `mdwn` hook is called for both `mdwn`
>> and `_mdwn` (assuming this is not already the case). I'm not convinced
>> that multi-format comments are a killer feature, though - part of the point
>> of this plugin, in my mind, is that it's less flexible than the full power
>> of ikiwiki and gives users fewer options. This could be construed
>> to be a feature, for people who don't care how flexible the technology is
>> and just want a simple way to leave a comment. The FormattingHelp page
>> assumes you're writing 100% Markdown in any case...
>>
>> Internal pages do too many things, perhaps: they suppress generation of
>> HTML pages, they disable editing over the web, and they have a different
>> namespace of htmlize hooks. I think the first two of those are useful
>> for this plugin, and the last is harmless; you seem to think the first
>> is useful, and the other two are harmful. --[[smcv]]
>> By the way, I think that who can post comments should be controllable by
>> the existing plugins opendiscussion, anonok, signinedit, and lockedit. Allowing
>> posting comments w/o any login, while a nice capability, can lead to
>> spam problems. So, use `check_canedit` as at least a first-level check?
>> --[[Joey]]
>> This plugin already uses `check_canedit`, but that function doesn't have a concept
>> of different actions. The hack I use is that when a user comments on, say, sandbox,
>> I call `check_canedit` for the pseudo-page "sandbox[postcomment]". The
>> special `postcomment(glob)` [[ikiwiki/pagespec]] returns true if the page ends with
>> "[postcomment]" and the part before (e.g. sandbox) matches the glob. So, you can
>> have postcomment(blog/*) or something. (Perhaps instead of taking a glob, postcomment
>> should take a pagespec, so you can have postcomment(link(tags/commentable))?)
>>
>> This is why `anonok_pages => 'postcomment(*)'` and `locked_pages => '!postcomment(*)'`
>> are necessary to allow anonymous and logged-in editing (respectively).
>>
>> This is ugly - one alternative would be to add `check_permission()` that takes a
>> page and a verb (create, edit, rename, remove and maybe comment are the ones I
>> can think of so far), use that, and port the plugins you mentioned to use that
>> API too. This plugin could either call `check_can("$page/comment1", 'create')` or
>> call `check_can($page, 'comment')`.
>>
>> One odd effect of the code structure I've used is that we check for the ability to
>> create the page before we actually know what page name we're going to use - when
>> posting the comment I just increment a number until I reach an unused one - so
>> either the code needs restructuring, or the permission check for 'create' would
>> always be for 'comment1' and never 'comment123'. --[[smcv]]
>> Another possibility is to just check for permission to edit (e.g.) `sandbox/comment1`.
>> However, this makes the "comments can only be created, not edited" feature completely
>> reliant on the fact that internal pages can't be edited. Perhaps there should be a
>> `editable_pages` pagespec, defaulting to `'*'`?
When using this plugin, you should also enable [[htmlscrubber]] and either [[htmltidy]]
or [[htmlbalance]]. Directives are filtered out by default, to avoid commenters slowing
down the wiki by causing time-consuming processing. As long as the recommended plugins
are enabled, comment authorship should hopefully be unforgeable by CGI users.
> I'm not sure that raw html should be a problem, as long as the
> htmlsanitizer and htmlbalanced plugins are enabled. I can see filtering
> out directives, as a special case. --[[Joey]]
>> Right, if I sanitize each post individually, with htmlscrubber and either htmltidy
>> or htmlbalance turned on, then there should be no way the user can forge a comment;
>> I was initially wary of allowing meta directives, but I think those are OK, as long
>> as the comment template puts the \[[!meta author]] at the *end*. Disallowing
>> directives is more a way to avoid commenters causing expensive processing than
>> anything else, at this point.
>>
>> I've rebased the plugin on master, made it sanitize individual posts' content
>> and removed the option to disallow raw HTML. Sanitizing individual posts before
>> they've been htmlized required me to preserve whitespace in the htmlbalance
>> plugin, so I did that. Alternatively, we could htmlize immediately and always
>> save out raw HTML? --[[smcv]]
>> There might be some use cases for other directives, such as img, in
>> comments.
>>
>> I don't know if meta is "safe" (ie, guaranteed to be inexpensive and not
>> allow users to do annoying things) or if it will continue to be in the
>> future. Hard to predict really, all that can be said with certainty is
>> all directives will contine to be inexpensive and safe enough that it's
>> sensible to allow users to (ab)use them on open wikis.
>> --[[Joey]]
When comments have been enabled generally, you still need to mark which pages
can have comments, by including the `\[[!comments]]` directive in them. By default,
this directive expands to a "post a comment" link plus an `\[[!inline]]` with
the comments. [This requirement has now been removed --[[smcv]]]
> I don't like this, because it's hard to explain to someone why they have
> to insert this into every post to their blog. Seems that the model used
> for discussion pages could work -- if comments are enabled, automatically
> add the comment posting form and comments to the end of each page.
> --[[Joey]]
>> I don't think I'd want comments on *every* page (particularly, not the
>> front page). Perhaps a pagespec in the setup file, where the default is "*"?
>> Then control freaks like me could use "link(tags/comments)" and tag pages
>> as allowing comments.
>>
>>> Yes, I think a pagespec is the way to go. --[[Joey]]
>>> Implemented --[[smcv]]
>>
>> The model used for discussion pages does require patching the existing
>> page template, which I was trying to avoid - I'm not convinced that having
>> every possible feature hard-coded there really scales (and obviously it's
>> rather annoying while this plugin is on a branch). --[[smcv]]
>>> Using the template would allow customising the html around the comments
>>> which seems like a good thing? --[[Joey]]
>>> The \[[!comments]] directive is already template-friendly - it expands to
>>> the contents of the template `comments_embed.tmpl`, possibly with the
>>> result of an \[[!inline]] appended. I should change `comments_embed.tmpl`
>>> so it uses a template variable `INLINE` for the inline result rather than
>>> having the perl code concatenate it, which would allow a bit more
>>> customization (whether the "post" link was before or after the inline).
>>> Even if you want comments in page.tmpl, keeping the separate comments_embed.tmpl
>>> and having a `COMMENTS` variable in page.tmpl might be the way forward,
>>> since the smaller each templates is, the easier it will be for users
>>> to maintain a patched set of templates. (I think so, anyway, based on what happens
>>> with dpkg prompts in Debian packages with monolithic vs split
>>> conffiles.) --[[smcv]]
>>> I've switched my branch to use page.tmpl instead; see what you think? --[[smcv]]
The plugin adds a new [[ikiwiki/PageSpec]] match type, `postcomment`, for use
with `anonok_pagespec` from the [[plugins/anonok]] plugin or `locked_pages` from
the [[plugins/lockedit]] plugin. Typical usage would be something like:
@ -183,20 +22,17 @@ to allow non-admin users to comment on pages, but not edit anything. You can als
to allow anonymous comments (the IP address will be used as the "author").
> This is still called postcomment, although I've renamed the rest of the plugin
> to comments as suggested on #ikiwiki --[[smcv]]
There are some global options for the setup file:
* comments_shown_pagespec: pages where comments will be displayed inline, e.g. `blog/*`
* `comments_shown_pagespec`: pages where comments will be displayed inline, e.g. `blog/*`
or `*/discussion`.
* comments_open_pagespec: pages where new comments can be posted, e.g.
* `comments_open_pagespec`: pages where new comments can be posted, e.g.
`blog/* and created_after(close_old_comments)` or `*/discussion`
* comments_pagename: if this is e.g. `comment_` (the default), then comments on the
* `comments_pagename`: if this is e.g. `comment_` (the default), then comments on the
[[sandbox]] will be called something like `sandbox/comment_12`
* comments_allowdirectives: if true (default false), comments may contain IkiWiki
* `comments_allowdirectives`: if true (default false), comments may contain IkiWiki
directives
* comments_commit: if true (default true), comments will be committed to the version
* `comments_commit`: if true (default true), comments will be committed to the version
control system
This plugin aims to close the [[todo]] item "[[todo/supporting_comments_via_disussion_pages]]",
@ -207,9 +43,10 @@ and is currently available from [[smcv]]'s git repository on git.pseudorandom.co
Known issues:
* Needs code review
* The access control via postcomment() is rather strange
* The access control via postcomment() is rather strange (see [[discussion]] for more details)
* There is some common code cargo-culted from other plugins (notably inline and editpage) which
should probably be shared
* Joey doesn't think it should necessarily use internal pages (see [[discussion]])
> I haven't done a detailed code review, but I will say I'm pleased you
> avoided re-implementing inline! --[[Joey]]
@ -222,3 +59,9 @@ Wishlist:
as someone else (even if anonymous comments are allowed, it'd be nice to be
able to choose to log in with a username or OpenID, like in Livejournal);
perhaps editpage needs this too
Fixed issues:
* Joey didn't think the `\[[!comments]]` directive was appropriate; comments now appear
on pages selected with a [[ikiwiki/pagespec]]
* Joey thought that raw HTML should always be allowed; it now is

View File

@ -0,0 +1,170 @@
## Why internal pages? (unresolved)
Comments are saved as internal pages, so they can never be edited through the CGI,
only by direct committers. Currently, comments are always in [[ikiwiki/markdown]].
> So, why do it this way, instead of using regular wiki pages in a
> namespace, such as `$page/comments/*`? Then you could use [[plugins/lockedit]] to
> limit editing of comments in more powerful ways. --[[Joey]]
>> Er... I suppose so. I'd assumed that these pages ought to only exist as inlines
>> rather than as individual pages (same reasoning as aggregated posts), though.
>>
>> lockedit is actually somewhat insufficient, since `check_canedit()`
>> doesn't distinguish between creation and editing; I'd have to continue to use
>> some sort of odd hack to allow creation but not editing.
>>
>> I also can't think of any circumstance where you'd want a user other than
>> admins (~= git committers) and possibly the commenter (who we can't check for
>> at the moment anyway, I don't think?) to be able to edit comments - I think
>> user expectations for something that looks like ordinary blog comments are
>> likely to include "others can't put words into my mouth".
>>
>> My other objection to using a namespace is that I'm not particularly happy about
>> plugins consuming arbitrary pieces of the wiki namespace - /discussion is bad
>> enough already. Indeed, this very page would accidentally get matched by rules
>> aiming to control comment-posting... :-) --[[smcv]]
>>> Thinking about it, perhaps one way to address this would be to have the suffix
>>> (e.g. whether commenting on Sandbox creates sandbox/comment1 or sandbox/c1 or
>>> what) be configurable by the wiki admin, in the same way that recentchanges has
>>> recentchangespage => 'recentchanges'? I'd like to see fewer hard-coded page
>>> names in general, really - it seems odd to me that shortcuts and smileys
>>> hard-code the name of the page to look at. Perhaps I could add
>>> discussionpage => 'discussion' too? --[[smcv]]
>>> (I've now implemented this in my branch. --[[smcv]])
>> The best reason to keep the pages internal seems to me to be that you
>> don't want the overhead of every comment spawning its own wiki page.
>> The worst problem with it though is that you have to assume the pages
>> are mdwn (or `default_pageext`) and not support other formats. --[[Joey]]
>>> Well, you could always have `comment1._mdwn`, `comment2._creole` etc. and
>>> alter the htmlize logic so that the `mdwn` hook is called for both `mdwn`
>>> and `_mdwn` (assuming this is not already the case). I'm not convinced
>>> that multi-format comments are a killer feature, though - part of the point
>>> of this plugin, in my mind, is that it's less flexible than the full power
>>> of ikiwiki and gives users fewer options. This could be construed
>>> to be a feature, for people who don't care how flexible the technology is
>>> and just want a simple way to leave a comment. The FormattingHelp page
>>> assumes you're writing 100% Markdown in any case...
>>>
>>> Internal pages do too many things, perhaps: they suppress generation of
>>> HTML pages, they disable editing over the web, and they have a different
>>> namespace of htmlize hooks. I think the first two of those are useful
>>> for this plugin, and the last is harmless; you seem to think the first
>>> is useful, and the other two are harmful. --[[smcv]]
## Access control (unresolved?)
By the way, I think that who can post comments should be controllable by
the existing plugins opendiscussion, anonok, signinedit, and lockedit. Allowing
posting comments w/o any login, while a nice capability, can lead to
spam problems. So, use `check_canedit` as at least a first-level check?
--[[Joey]]
> This plugin already uses `check_canedit`, but that function doesn't have a concept
> of different actions. The hack I use is that when a user comments on, say, sandbox,
> I call `check_canedit` for the pseudo-page "sandbox[postcomment]". The
> special `postcomment(glob)` [[ikiwiki/pagespec]] returns true if the page ends with
> "[postcomment]" and the part before (e.g. sandbox) matches the glob. So, you can
> have postcomment(blog/*) or something. (Perhaps instead of taking a glob, postcomment
> should take a pagespec, so you can have postcomment(link(tags/commentable))?)
>
> This is why `anonok_pages => 'postcomment(*)'` and `locked_pages => '!postcomment(*)'`
> are necessary to allow anonymous and logged-in editing (respectively).
>
> This is ugly - one alternative would be to add `check_permission()` that takes a
> page and a verb (create, edit, rename, remove and maybe comment are the ones I
> can think of so far), use that, and port the plugins you mentioned to use that
> API too. This plugin could either call `check_can("$page/comment1", 'create')` or
> call `check_can($page, 'comment')`.
>
> One odd effect of the code structure I've used is that we check for the ability to
> create the page before we actually know what page name we're going to use - when
> posting the comment I just increment a number until I reach an unused one - so
> either the code needs restructuring, or the permission check for 'create' would
> always be for 'comment1' and never 'comment123'.
> Another possibility is to just check for permission to edit (e.g.) `sandbox/comment1`.
> However, this makes the "comments can only be created, not edited" feature completely
> reliant on the fact that internal pages can't be edited. Perhaps there should be a
> `editable_pages` pagespec, defaulting to `'*'`? --[[smcv]]
## comments directive vs global setting (resolved?)
When comments have been enabled generally, you still need to mark which pages
can have comments, by including the `\[[!comments]]` directive in them. By default,
this directive expands to a "post a comment" link plus an `\[[!inline]]` with
the comments. [This requirement has now been removed --[[smcv]]]
> I don't like this, because it's hard to explain to someone why they have
> to insert this into every post to their blog. Seems that the model used
> for discussion pages could work -- if comments are enabled, automatically
> add the comment posting form and comments to the end of each page.
> --[[Joey]]
>> I don't think I'd want comments on *every* page (particularly, not the
>> front page). Perhaps a pagespec in the setup file, where the default is "*"?
>> Then control freaks like me could use "link(tags/comments)" and tag pages
>> as allowing comments.
>>
>>> Yes, I think a pagespec is the way to go. --[[Joey]]
>>>> Implemented --[[smcv]]
>>
>> The model used for discussion pages does require patching the existing
>> page template, which I was trying to avoid - I'm not convinced that having
>> every possible feature hard-coded there really scales (and obviously it's
>> rather annoying while this plugin is on a branch). --[[smcv]]
>>> Using the template would allow customising the html around the comments
>>> which seems like a good thing? --[[Joey]]
>>>> The \[[!comments]] directive is already template-friendly - it expands to
>>>> the contents of the template `comments_embed.tmpl`, possibly with the
>>>> result of an \[[!inline]] appended. I should change `comments_embed.tmpl`
>>>> so it uses a template variable `INLINE` for the inline result rather than
>>>> having the perl code concatenate it, which would allow a bit more
>>>> customization (whether the "post" link was before or after the inline).
>>>> Even if you want comments in page.tmpl, keeping the separate comments_embed.tmpl
>>>> and having a `COMMENTS` variable in page.tmpl might be the way forward,
>>>> since the smaller each templates is, the easier it will be for users
>>>> to maintain a patched set of templates. (I think so, anyway, based on what happens
>>>> with dpkg prompts in Debian packages with monolithic vs split
>>>> conffiles.) --[[smcv]]
>>>>> I've switched my branch to use page.tmpl instead; see what you think? --[[smcv]]
## Raw HTML (resolved?)
Raw HTML was not initially allowed by default (this was configurable).
> I'm not sure that raw html should be a problem, as long as the
> htmlsanitizer and htmlbalanced plugins are enabled. I can see filtering
> out directives, as a special case. --[[Joey]]
>> Right, if I sanitize each post individually, with htmlscrubber and either htmltidy
>> or htmlbalance turned on, then there should be no way the user can forge a comment;
>> I was initially wary of allowing meta directives, but I think those are OK, as long
>> as the comment template puts the \[[!meta author]] at the *end*. Disallowing
>> directives is more a way to avoid commenters causing expensive processing than
>> anything else, at this point.
>>
>> I've rebased the plugin on master, made it sanitize individual posts' content
>> and removed the option to disallow raw HTML. Sanitizing individual posts before
>> they've been htmlized required me to preserve whitespace in the htmlbalance
>> plugin, so I did that. Alternatively, we could htmlize immediately and always
>> save out raw HTML? --[[smcv]]
>>> There might be some use cases for other directives, such as img, in
>>> comments.
>>>
>>> I don't know if meta is "safe" (ie, guaranteed to be inexpensive and not
>>> allow users to do annoying things) or if it will continue to be in the
>>> future. Hard to predict really, all that can be said with certainty is
>>> all directives will contine to be inexpensive and safe enough that it's
>>> sensible to allow users to (ab)use them on open wikis.
>>> --[[Joey]]

View File

@ -138,3 +138,10 @@ Thanks for your response. You're right. Ubuntu does have ikiwiki, except that it
Anyway, I think I might be able to install it from the tarball I downloaded. I've been reading the discussions, had a look at your screencasts, etc. I will give it another bash. -- [[WillDioneda]]
----
How do I set up cgi editing? In setup I have:
* cgiurl => 'http://wiki.had.co.nz/edit.cgi'
* cgi_wrapper => 'edit.cgi'
But I don't get an edit link on my pages? What am I doing wrong?

View File

@ -13,3 +13,12 @@ As a side note, the accompanying proxy.py might better be placed into some direc
> If someone can show how to do so without needing a Setup.py and all the
> pain that using one entails.. --[[Joey]]
>> At the very least I don't think proxy.py should be on the `sys.path`
>> under its current name. If it was renamed to ikiwiki_proxy or some such,
>> possibly; but I think it's more appropriate to have it in an
>> ikiwiki-specific directory (a "private module") since it's not useful for
>> anything outside ikiwiki, and putting it in the same directory as the
>> external plugins means it's automatically in their `sys.path` without
>> needing special configuration. --[[smcv]]
>> (a mostly-inactive member of Debian's Python modules packaging team)

View File

@ -1,3 +1,3 @@
The ikiwiki-w3m.cgi script is installed (hard-coded) into /usr/lib/w3m/cgi-bin/. On Fedora however, the w3m package expects it in /usr/libexec/w3m/cgi-bin. So, it would be nice if the destination for this script could be configured.
The `ikiwiki-w3m.cgi` script is installed (hard-coded) into `/usr/lib/w3m/cgi-bin`. On Fedora however, the w3m package expects it in `/usr/libexec/w3m/cgi-bin`. So, it would be nice if the destination for this script could be configured.
> You can use W3M_CGI_BIN now. [[done]] --[[Joey]]
> You can use `W3M_CGI_BIN now`. [[done]] --[[Joey]]