Re: Problems with comments.py

On Jun 9, 2005, at 6:49 AM, will guaraldi wrote:

> On Wed, 8 Jun 2005, Nathaniel Gray wrote:
>
>> 2.  The new way of having story.html displayed for comments makes  
>> it impossible to put status/error messages at the top of the page  
>> where they're easiest to see.  So the user has to scroll down  
>> beyond the story to even know that there was a problem with a  
>> comment.  It was also annoying that this behavior changed without  
>> notice between 1.0 and 1.2.1.
>>
>
> The old way would stomp on the existing story template which caused  
> other problems.
>
> You could put in an anchor that will scroll the page down to where  
> the comment error is immediately upon posting.  I would suggest  
> using that.

Any hints on how I get the browser to do that?  Sorry, I'm not much  
of a web programmer.  :-/

> In terms of notice, I'm pretty sure there's notice in several  
> places. This was a known not-backwards-compatible behavior change  
> of the comments plugin and I thought I had made sure that there was  
> ample notice.  Having said that, I don't see anything anywhere.  I  
> think I totally dropped the ball.  I thought I had a README just  
> like I did with the PyBlosxom core packages.

It's possible I could have missed the notice, but I thought I read  
the available docs.  I know I looked at the changelogs on SF.

>> 3.  It's hard to style status/error messages.  If a message is  
>> output, $comment_message should expand to a <div  
>> style="commenterrormsg">...</div> block.  If no message is output  
>> it should expand to an empty string.  That makes it easy to  
>> incorporate the error message output in your flavour files without  
>> having them take up space, and it makes them easier to style as well.

How about this one?  I really want to put my error message in a big  
red box, but without having the error message in a div I end up with  
a red line in the non-error case.

>> 6.  I've added a "magic word" feature to comments.py that requires  
>> the user to type some magic word to validate that they're not a  
>> spammer. It's really simple, not dynamic, not industrial-strength,  
>> but if everybody out there chose a different word and a different  
>> way of *asking for* the word it would make comment spamming much  
>> harder.  Is this something you'd be interested in integrating?
>>
>
> I've been doing a lot of work on the PyBlosxom manual to covers  
> issues. I tossed in a chapter on comments last week or so which  
> talks about dealing with comment spam (though much of the material  
> is missing):
>
>    http://pyblosxom.sourceforge.net/1.2/manual/c848.html
>
> The comments plugin uses a comment rejection callback to allow you  
> to have other plugins deal with the business logic for how you want  
> to reduce spam.

Ah, very nice!  The only problem I can see is there's no way for the  
plugin to report *why* it rejected the comment.  This is probably  
obvious to the programmer/blog owner, but not always to the  
commenter.  It would be nice if the plugin could return:
     (True, "Incorrect magic word!")

> On the PyBlosxom site, we use the nospam plugin (check the mailing  
> lists and registry for details) to reduce comment spam.  On my  
> site, I reject all comments that have words like "casino" and  
> "blackjack" in them using my wbgcomment_blacklist plugin and then  
> all other comments go into draft status to be manually approved.

I'm not thrilled by word-based blacklisting.  It leads to too many  
false positives.

> So, we're not interested in adding that to the comments plugin  
> directly, but you could write a comment-rejection plugin based on  
> what you're talking about and the nospam plugin does something  
> similar to what you're talking about.

Ok, I've written one.  How do I submit it for inclusion in the  
registry and/or contrib package?

>> Ok, I guess that's it for now.  I'm sorry if I came off as  
>> sounding negative. I actually like pb a lot and I'm grateful for  
>> the time you guys spend on it. If you agree with the things I've  
>> said above but don't have the time to fix them then I can put  
>> together patches.
>>
>
> I'm ok with negativity--these issues you're bringing up are  
> definitely issues.
>
> If you could work up a patch for issues 1 and 5, that would help a  
> lot.

Ok, it's attached.

> I think we're definitely interested in thoughts on how to  
> restructure the system to better handle error messages from plugins.
>
> As you state, there are two flavors of error messages: ones that  
> need to go back to the user and ones that should only be seen by  
> the blog owner. The latter should go to the log files, but our  
> logging is still in somewhat of a shambles.  Steven committed some  
> logging fixes to the DEVEL branch, but I haven't brought them over  
> into the 1.2 branch because I'm just doing bug fixes right now and  
> don't have time to do anything else. Those are conversations for  
> the pyblosxom-devel mailing list.

True, I should subscribe over there.  I get *so* tired of subscribing  
to random mailing lists, though.  I must have 30 lists I'm subscribed  
to with delivery disabled just so I can post bug reports.  Mailman  
needs to integrate some sort of single-sign-on solution.

> Inconveniently, I've got HUGE life changes in the next two weeks  
> and then I'm on vacation until July.  I'll try to slam-dunk some of  
> the things I'm sitting on over the next few days so I'm not leaving  
> anyone high and dry.

Good luck!

Cheers,
-n8

--
 >>>-- Nathaniel Gray -- Caltech Computer Science ------>
 >>>-- Mojave Project -- http://mojave.cs.caltech.edu -->

--- ../contrib-1.2.1/plugins/comments/plugins/comments.py	2005-04-12 12:12:22.000000000 -0700
+++ comments.py	2005-06-09 16:46:26.000000000 -0700
@@ -314,25 +314,34 @@
                                 donefunc=lambda x:x)
     if reject == 1:
         return "Comment rejected."
+    
+    def makeXMLField(name, field):
+        return "<"+name+">"+cgi.escape(field[name])+"</"+name+">\n";
+    
+    # Transform before writing, to reduce the chance of file corruption
+    xtitle = makeXMLField('title',comment)
+    xauthor = makeXMLField('author',comment)
+    xlink = makeXMLField('link',comment)
+    xemail = makeXMLField('email',comment)
+    xsource = makeXMLField('source',comment)
+    xpubDate = makeXMLField('pubDate',comment)
+    xdescription = makeXMLField('description',comment)
 
     try :
         cfile = codecs.open(cfn, "w", encoding)
     except IOError:
         tools.log("Couldn't open comment file %s for writing" % cfn)
-        return
-    
-    def makeXMLField(name, field):
-        return "<"+name+">"+cgi.escape(field[name])+"</"+name+">\n";
+        return "Internal error: Your comment could not be saved."
     
     cfile.write('<?xml version="1.0" encoding="%s"?>\n' % encoding)
     cfile.write("<item>\n")
-    cfile.write(makeXMLField('title',comment))
-    cfile.write(makeXMLField('author',comment))
-    cfile.write(makeXMLField('link',comment))
-    cfile.write(makeXMLField('email',comment))
-    cfile.write(makeXMLField('source',comment))
-    cfile.write(makeXMLField('pubDate',comment))
-    cfile.write(makeXMLField('description',comment))
+    cfile.write(xtitle)
+    cfile.write(xauthor)
+    cfile.write(xlink)
+    cfile.write(xemail)
+    cfile.write(xsource)
+    cfile.write(xpubDate)
+    cfile.write(xdescription)
     cfile.write("</item>\n")
     cfile.close()
  
@@ -343,7 +352,7 @@
         latest = open(latestFilename,"w")
     except IOError:
         tools.log("Couldn't open latest comment pickle for writing")
-        return "Couldn't open latest comment pickle for writing."
+        return "Internal error: Your comment could not be saved."
     else:
         modTime = float(comment['pubDate'])
 
@@ -354,19 +363,18 @@
         # should log or e-mail
         if latest:
             latest.close()
-        return "Unable to dump latest comment pickle."
+        tools.log("Unable to dump latest comment pickle.")
+        return "Internal error: Your comment may not have been saved."
 
-    ret = ""
-    
     if config.has_key('comment_smtp_server') and \
        config.has_key('comment_smtp_to'):
-        ret = send_email(config, entry, comment, cdir, cfn)
+        send_email(config, entry, comment, cdir, cfn)
 
     # figure out if the comment was submitted as a draft
     if config["comment_ext"] != config["comment_draft_ext"]:
-       return ret + "Comment was submitted for approval.  Thanks!"
+       return "Comment was submitted for approval.  Thanks!"
 
-    return ret + "Comment submitted.  Thanks!"
+    return "Comment submitted.  Thanks!"
 
 def send_email(config, entry, comment, comment_dir, comment_filename):
     """Send an email to the blog owner on a new comment
@@ -418,9 +426,6 @@
         server.quit()
     except Exception, e:
         tools.log("Error sending mail: %s" % e)
-        return "Error sending mail: %s" % e
-
-    return ""
 
 def clean_author(s):
     """