[drupal-devel] [feature] Hooks for comment enhancements
chx
drupal-devel at drupal.org
Mon Sep 12 21:55:49 UTC 2005
Issue status update for
http://drupal.org/node/28255
Post a follow up:
http://drupal.org/project/comments/add/28255
Project: Drupal
Version: cvs
Component: comment.module
Category: feature requests
Priority: critical
Assigned to: Anonymous
Reported by: Eaton
Updated by: chx
-Status: patch (code needs review)
+Status: patch (ready to be committed)
I downloaded the patch and checked it against my comment upload module
and the thing works.
chx
Previous comments:
------------------------------------------------------------------------
Fri, 05 Aug 2005 05:24:26 +0000 : Eaton
Attachment: http://drupal.org/files/issues/comment.module_10.patch (2.05 KB)
I've been working on a couple modules that modify and enhance the
commenting system (file attachments in comments, and custom comment
validation for specific nodetypes). Another patch
(http://drupal.org/node/14708) would add limited form and validation
customizatoin, but this one adds nodeapi-style form_pre, form_post,
validate, and view hooks.
It's my first patch offered to the Drupal core, hope it's useful.
------------------------------------------------------------------------
Sat, 13 Aug 2005 13:54:26 +0000 : koorneef
When applied to CVS (2005-08-13):
(lucas at bsd1)/www/koorneef/test.koorneef.net/modules$ patch <
comment.module_10.patch
Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: modules/comment.module
|===================================================================
|RCS file: /cvs/drupal/drupal/modules/comment.module,v
|retrieving revision 1.364
|diff -u -r1.364 comment.module
|--- modules/comment.module 1 Aug 2005 05:14:05 -0000 1.364
|+++ modules/comment.module 5 Aug 2005 05:19:14 -0000
--------------------------
Patching file comment.module using Plan A...
Hunk #1 failed at 434.
Hunk #2 failed at 1417.
Hunk #3 failed at 1427.
Hunk #4 failed at 1469.
Hunk #5 succeeded at 1699 with fuzz 2.
4 out of 5 hunks failed--saving rejects to comment.module.rej
done
I'll see if I can update the patch ...
------------------------------------------------------------------------
Sat, 13 Aug 2005 14:34:09 +0000 : koorneef
Attachment: http://drupal.org/files/issues/comment.module_11_0.patch (2.05 KB)
Found the problem: the patch file had DOS format line-endings. Corrected
in new version: 11.
Patch applies, but captcha is not appearing on comment form (it did
with the older patches, see http://drupal.org/node/14708). Have to
investigate further ....
------------------------------------------------------------------------
Sun, 14 Aug 2005 04:48:14 +0000 : Eaton
after some investigation it appears that captcha is expecting a general
'form' $op for the hook, rathre than the 'form_pre' and 'form_post' ops
that this patch supplies.
I added the pre and post operations to mirror the nodeapi form hooks,
I'm not sure that a third general 'form' op would be a good idea. Any
thoughts?
------------------------------------------------------------------------
Tue, 16 Aug 2005 18:14:25 +0000 : moshe weitzman
i think consistency with nodeapi is important, so no 'form' operation
... this patch looks simple and useful to me.
------------------------------------------------------------------------
Tue, 16 Aug 2005 18:24:43 +0000 : arnabdotorg
+1 for consistency; we can (I will) modify captcha to form_post once
this hits core.
------------------------------------------------------------------------
Wed, 17 Aug 2005 12:53:03 +0000 : Eaton
Attachment: http://drupal.org/files/issues/comment.module_14.patch (2.75 KB)
An updated version of the patch with some tweaks. First, the 'form_pre'
and 'form_post' operations have been changed to 'form pre' and 'form
post' to reflect the nodeapi naming conventions.
Second, a 'form param' hook has been added, mirroring nodapi's form
param hook. This allows modules that modify the comment form to
integrate file uploading and other niceties.
------------------------------------------------------------------------
Wed, 17 Aug 2005 12:57:30 +0000 : chx
In the beginning comments were supposed to be simple texts.. If you add
files, how do you display without 'load' and 'view' operations? If you
add 'load' and 'view', then why not create a 'comment' node type?
------------------------------------------------------------------------
Wed, 17 Aug 2005 15:01:22 +0000 : Eaton
Your point is a good one. I'd like to work on a comprehensive comment
node type rewrite, but this was a 'good enough' solution that helped
out existing modules like captcha and doesn't break existing
comment-based solutions.
the comment file-attachment stuff is just a possible use of some of the
hooks; right now it's impossible. in the future a much cleaner solution
would be nice. i've also used the validation hooks in implementing
debate.module and microfiction.module.
------------------------------------------------------------------------
Thu, 18 Aug 2005 14:51:32 +0000 : Eaton
I've checked in a proof of concept module called image_thread that
relies on this patch to provide image uploading in a node's comments.
I'm using it on a small site for photoshop image contests. That module
is rough, but it gives an example of how the new hooks might be used to
provide more complex enhancements.
http://cvs.drupal.org/viewcvs/drupal/contributions/modules/image_thread/
------------------------------------------------------------------------
Sun, 21 Aug 2005 14:46:10 +0000 : chx
Important notice: if this gets in, then project module can be overhauled
to use the comment system!
------------------------------------------------------------------------
Sat, 27 Aug 2005 00:46:42 +0000 : Eaton
Anyone willing to review this for 4.7 inclusion? I have high hopes...
------------------------------------------------------------------------
Sat, 27 Aug 2005 01:04:22 +0000 : nedjo
qualified +1: I have looked over but not applied this patch.
I like the added functionality. In general, it helps move us toward
parallelism between different object types--which is good. My feeling
is: unless there's a good reason *not* to expose a particular Drupal
object type (comment, term, vocabulary, user, for a start) to
nodeapi-like hooks, we should do so. If someone wants to do it with
nodes, likely there's a good use case with other object types. Case in
point: I'm continually frustrated with the inability to act on taxonomy
term views (my patch, http://drupal.org/node/10399, a year ago, went
nowhere, but maybe I should revive it).
Why not just use a node type? Because nodes are for different things
than comments--twisting them into comments is bad practice.
The potential to use this for project.module, as suggested, is an added
bonus.
------------------------------------------------------------------------
Sat, 27 Aug 2005 15:14:06 +0000 : Cvbge
A question about last patch: in db_query("UPDATE
{node_comment_statistics} SET comment_count = %d,
last_comment_timestamp = %d, last_comment_name = '%s', last_comment_uid
= %d WHERE nid = %d", 0, NULL, 0, 0, $nid); why do you use NULL for
last_comment_timestamp if it'd %d? It will be changed to '0', so if you
want to use reall sql NULL type you need to put it directly.
------------------------------------------------------------------------
Sat, 27 Aug 2005 15:59:00 +0000 : Eaton
Actually, that line isn't one of the added or changed lines, it was in
the CVS copy that I patched from. I can take a look to see if the CVS
version has changed dramatically since I created the patch -- I haven't
had a chance to update in a couple of days.
------------------------------------------------------------------------
Sat, 27 Aug 2005 16:04:27 +0000 : Cvbge
Ah, didn't pay attention to the fact that it's not changed in patch...
sorry about that. Happens sometimes to me ;)
------------------------------------------------------------------------
Mon, 29 Aug 2005 15:14:51 +0000 : Eaton
To anyone interested in seeing these hooks go into core, taking a bit of
time to apply it and give feedback would be great. Some example modules
that demonstrate the new hooks are at:
http://drupal.org/node/28163 (a module with optional wordcount
restrictions on comments)
http://drupal.org/node/28407(a module that prevents individuals from
commenting until they have 'joined' a given debate)
http://cvs.drupal.org/viewcvs/drupal/contributions/modules/image_thread/
(a module that lets users upload images in the comments of a node)
These modules are rough, but give examples of how the hooks might be
used.
------------------------------------------------------------------------
Thu, 01 Sep 2005 21:23:01 +0000 : Poromenos at www.poromenos.org
Any hope of a patch for 4.6.3? This is really useful.
------------------------------------------------------------------------
Fri, 02 Sep 2005 01:33:27 +0000 : Eaton
Attachment: http://drupal.org/files/issues/comment.module.4-6-3.patch (2.36 KB)
Attached is a patch against 4.6.3. Hopefully, the HEAD version of it
will be approved. Feedback is much appreciated...
------------------------------------------------------------------------
Wed, 07 Sep 2005 18:28:20 +0000 : meba
your patch for 4.6.3 is again in DOS format...
also, none of these patches works for 4.6.2 :(
------------------------------------------------------------------------
Wed, 07 Sep 2005 18:34:44 +0000 : Eaton
meba, apologies regarding the windows format. The patch doesn't apply
against 4.6.2 beause there were changes in the 4.6.3 branch to
comments.module.
To be honest, I'm not going to be putting energy into a 4.6 patch now.
I'm hoping to get some testing done and push for inclusion in HEAD, so
that it can be made 'standard'. Once that happens, I think it can
safely be backported to 4.6.x.
------------------------------------------------------------------------
Thu, 08 Sep 2005 00:02:30 +0000 : michaelsb
I'm working on task.module and I wanted to provide task updates through
comments. This patch did the trick and was such a big time saver
(thanks to killes for recommending it).
However, I do have a feature request for this patch. I'd be nice if you
could pass $comment by reference, so that I could use 'validate' to
validate and update certain values. This way when 'view' is executed
$comment will already have the validated data and I want have to call
my hook_validate function again. This will mimic the way node hooks
work, for example hook_validate passes $node by reference.
Also, update your patch to include the newest comment.module changes.
The main one being the removal of ?> at the end of the file.
All in all, I give this patch a +1 for it's usefulness. Thanks!
------------------------------------------------------------------------
Thu, 08 Sep 2005 03:49:55 +0000 : Eaton
Attachment: http://drupal.org/files/issues/comment.module_17.patch (3.48 KB)
New version attached. This one applies cleanly against the current HEAD,
and passes the comment by reference so various handlers don't have to do
double-duty.
------------------------------------------------------------------------
Thu, 08 Sep 2005 19:13:25 +0000 : Eaton
Attachment: http://drupal.org/files/issues/comment.module_18.patch (3.47 KB)
A discussion with UnConeD and chx indicates that passing by reference is
the norm in node.module, but is not kosher with te comment.module's way
of doing things. This version includes all other tweaks but does NOT
pass by ref.
------------------------------------------------------------------------
Mon, 12 Sep 2005 16:46:49 +0000 : Eaton
Attachment: http://drupal.org/files/issues/comment.module_19.patch (2.67 KB)
Fixing a silly bug in the form param code that would've nuked any
'destination' parameter.
------------------------------------------------------------------------
Mon, 12 Sep 2005 17:08:48 +0000 : Eaton
Attachment: http://drupal.org/files/issues/comment.module_20.patch (2.7 KB)
Er, rather, fixed in THIS version. Properly keeps all the assorted
values of the attributes array.
More information about the drupal-devel
mailing list