[drupal-devel] [feature] Create extra form elements for comment form
stefan nagtegaal
drupal-devel at drupal.org
Sat Jan 22 09:44:58 UTC 2005
Project: Drupal
Version: cvs
Component: comment.module
Category: feature requests
Priority: normal
Assigned to: nysus
Reported by: nysus
Updated by: stefan nagtegaal
Status: patch
@ TDobes: The other issue is marked as 'duplicate', while here is the
most interesting discussion going on about contentapi() vs.
nodeapi()/commentapi()..
Dries, what is you opinion about this? What needs to be done to get a
patch like this in core?
- Should we code the proposed contentapi()?
- Another implementation?
I really would like this in before we freeze HEAD, so i'm willing to
cooperate in this..
stefan nagtegaal
Previous comments:
------------------------------------------------------------------------
November 26, 2004 - 21:33 : nysus
Like node forms which allow modules to use the nodeapi hook to prepend
and append form elements to the node form, this simple change lets
modules do the same for the comment form. Like node form, it also
relies on the nodeapi hook.
I plan to use it for an improvement to the subscriptions module that I
have written which allows users to subscribe to individual comments
without having to subscribe to an entire node.
This change to the core was recommended by Moshe Weitzam found here:
http://lists.drupal.org/archives/drupal-devel/2004-11/msg00256.html
Thanks for the tip, Moshe!
------------------------------------------------------------------------
November 26, 2004 - 21:34 : nysus
Attachment: http://drupal.org/files/issues/comment_8.patch (278 bytes)
Oops, would help to attach the patch, eh?
------------------------------------------------------------------------
November 26, 2004 - 22:04 : TDobes
A few things:
1. Please use the unified diff format for making patches -- see
http://drupal.org/patch
2. I believe the suggestion was to add these features to hook_comment
[1], not hook_nodeapi. nodeapi is just for nodes... comment is just
for comments.
3. (not really related to yoru patch) Wouldn't it make more sense to
have a hook_commentapi rather than a hook_comment? It seems like this
would be more consistent with the naming of hook_nodeapi.
4. Many of the nodeapi $op values [2] could also apply to comments.
Adding these to a commentapi could really open a lot of nice
possibilities for modules.
[1] http://drupaldocs.org/api/head/function/hook_comment
[2] http://drupaldocs.org/api/head/function/hook_nodeapi
------------------------------------------------------------------------
November 26, 2004 - 22:36 : nysus
TDobes,
I don't know if you read the thread from the previous discussion, but
there I pointed out that I didn't think the hook_comment seemed like
the right function to modify.
As far as creating a commentapi function, I thought about that. But I
wondered if adding all the code necessary to support it was really
worth it.
You can use the pre-existing nodeapi to accomplish the same exact
thing. And you won't have to create a new commentapi function in the
comment.module and you won't have to create a new function in each of
the modules that wants to use it. All you have to do is slap another
case in the module's _nodeapi hook.
So, you've got this in the comment.module's theme_comment_form()
function:
// Append extra comment form elements
$form .= implode('', node_invoke_nodeapi($edit, 'comment form
post'));
And then in the subscription.module, you've got the following in the
subscription_nodeapi() function:
I understand your concern about maintaining naming conventions.
Perhaps the nodeapi hook should be given a new name since it can have
functionality beyond nodes.
PS: Yeah, I know about patching with -u switch, just forgot.
------------------------------------------------------------------------
November 26, 2004 - 23:01 : TDobes
I did read the thread that you linked to. At the moment (before your
patch), hook_nodeapi is ONLY concerned with nodes. I believe it should
stay that way.
It is not that much more work for module authors to add a _comment (or
_commentapi if we rename it) function to their module. On the other
hand, it would be exceptionally confusing for people new to Drupal if
we placed comment functions in a node hook.
If we do decide to combine both hooks, I agree with you that nodeapi
should be renamed. However, for the time being, I do not feel this is
necessary.
So... I agree with you that this would be a nice feature. However, I'd
prefer a larger, more logical patch (adding to hook_comment) to a small
and confusing patch (sticking this in hook_nodeapi, where it doesn't
belong, IMO). But that's just my opinion... we'll have to see what
others think.
------------------------------------------------------------------------
November 26, 2004 - 23:03 : nysus
TDobes,
OK, after rereading what you wrote and thinking about this some more, I
think I may see a small advantage to having a separate hook_comment
function. You are saying you could use the same op code string for
both the hook_comment and nodeapi, right?
But how about if the op code string were appended in some standardized
way so the module _nodeapi hook could still be used? For example, you
could prepend 'comment' to each of the codes for each operation that
dealt with comments.
So you'd have 'add' for when a node is added use 'comment add' for when
a comment is added. 'insert' would be used for nodes and 'comment
insert' for when comments are inserted.
I guess it all boils down to the larger question I raised earler: Do we
really need a separate _comment hook?
------------------------------------------------------------------------
November 26, 2004 - 23:06 : nysus
Right, let's see what others think and go from there. Anyone?
------------------------------------------------------------------------
November 26, 2004 - 23:19 : Steven
Hook_comment /is/ comment API. Renaming it to hook_commentapi seems like
a good consistency idea. Putting all this inside hook_nodeapi is
unacceptable and unclean.
------------------------------------------------------------------------
November 27, 2004 - 00:10 : moshe weitzman
the best idea proposed by jhriggs was to combine nodeapi, commentapi,
and user hooks into one contentapi hook. see
http://lists.drupal.org/archives/drupal-devel/2004-07/msg00614.html
in that msg, jhriggs prefers a single contentapi() hook over multiple
$op specific hooks. Thats my slight preference as well.
------------------------------------------------------------------------
November 27, 2004 - 00:23 : Steven
The contentapi puts us back in the discussion of "what is content?". Are
users content? What about user profiles? etc.
I don't think a generalized contentapi hook will help us though. A
module will rarely affect nodes, comments and users at the same time.
Almost all modules will implement contentapi and have a if ($type ==
'node') or if ($type == 'comment') at the beginning. This makes the
code more complicated than it should. It is also bad from a performance
point of view to call every module which does something with nodes,
comments or users when an operation is performed on any of them.
------------------------------------------------------------------------
November 27, 2004 - 04:51 : nysus
After a little more consideration and after reading some of the comments
here, I'll just recode this so that it uses the hook_comment. It just
doesn't make sense to try to push through a fundamental change to
Drupal's API to accomodate this small patch.
------------------------------------------------------------------------
November 27, 2004 - 05:37 : nysus
Attachment: http://drupal.org/files/issues/comment_9.patch (876 bytes)
OK, here's a second revision that works with hook_comment() instead.
------------------------------------------------------------------------
November 27, 2004 - 09:45 : Dries
"Wouldn't it make more sense to have a hook_commentapi rather than a
hook_comment? It seems like this would be more consistent with the
naming of hook_nodeapi."
No, but it would make sense to rename hook_nodeapi to hook_node.
------------------------------------------------------------------------
November 29, 2004 - 18:01 : jhriggs
I still like the _contentapi() idea. I have discussed it several times
with Kjartan, and although we have a few differences in opinion as to
exactly how it should be coded, we both agree that it would be useful
functionality. It does raise the "What is content?" question as Steven
points out. Perhaps another name would be more appropriate.
I do see tremendous benefit in having a single entry-point for this
type of extensibility, though. Say for example that I want to append
some type of disclaimer or a verification step (a la captcha or a
similar module I wrote awhile back) in every submission form, be it
node, comment, poll vote, user registration, profile, etc. I would
only have to handle it in one place. Every time I receive a 'form pre'
or 'form post', I can insert my content...or I can handle certain cases
differently if desired (based on the type of content).
Additionally, any module would be able to benefit from the logic.
Instead of having somewhat overlapping _nodeapi(), _user(), and
_comment() functionality like we do now (or will have even more of with
the patch proposed here), we would have one invocation function that any
module can use for any type of content that they define (i.e. user in
user module, profile in profile.module, node in node.module, comment in
comment.module, poll vote form in poll.module, etc.).
Finally, as I have also discussed with Kjartan, functionality like this
would mean that project.module would not have to use its own comment
system (lots of duplicated code). It would be able to use the enhanced
form-insertion and verification available through the hook.
If there is sufficient interest, I will revive/rework the patch that I
have started several times...
------------------------------------------------------------------------
December 24, 2004 - 07:48 : TDobes
There's a similar discussion going on in another issue [3]... one of
these should be marked as a duplicate.
[3] http://drupal.org/node/14708
--
View: http://drupal.org/node/13539
Edit: http://drupal.org/project/comments/add/13539
More information about the drupal-devel
mailing list