CVS edit link for siliconmeadow
I have a module I've called "EmptyParagraphKiller". I have found that many sites have a need to deal with regular users who often hit the return key twice at the end of a paragraph. For most modern design web sites, where the layout handles the paragraphhttps://github.com/siliconmeadow/emptyparagraphkiller
I have searched d.o and can not find a module that does this task. I've posted to the "Contributed Modules Ideas" group here:
http://groups.drupal.org/node/107914
I've also been asked if I was available to help co-maintain the clients module and now have the spare capacity and approval by my company to contribute what I can to the community.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | emptyparagraphkiller.tar_.gz | 1.86 KB | siliconmeadow |
| #13 | emptyparagraphkiller.tar_.gz | 1.85 KB | siliconmeadow |
| #8 | 978750_01_emptyparagraphkiller.patch | 2.77 KB | siliconmeadow |
| #7 | 978750_01_emptyparagraphkiller.patch | 2.77 KB | siliconmeadow |
| #1 | emptyparagraphkiller.tar_.gz | 1.78 KB | siliconmeadow |
Comments
Comment #1
siliconmeadow commentedHere is the EmptyParagraphKiller module.
Comment #2
siliconmeadow commentedCo-maintainer request:
http://drupal.org/node/978746
Comment #3
manarth commentedThe EmptyParagraphKiller module is well-written, conforms to coding standards and is very clear and easy to understand. If there are any other modules which also provide this feature, I've not come across them.
The co-maintainer request has also been approved by the maintainer of the Web service clients module.
Comment #4
joachim commented> $text = preg_replace('#
]*>(\s| ?)*
#', '', $text);
This seems to suppose you're going in *after* the HTML linebreak filter. What if you went before -- the regex would be simpler.
> return t('Empty Paragraph Killer - when entering more than one carriage return, only the first will be honored.');
> ...
> return t('When entering more than one carriage return, only the first will be honored. Users often hit the
Filter tips should be addressed to the user about to type. Compare the voice of these with the filters in core.
> name = Empty Paragraph Killer
IIRC module names are in sentence case.
> EmptyParagraphKiller is a filter module.
Match the name given in the info file.
> such that it never alters the user’s original data - it's
???
> distructive
typo -- 'destructive'
Also, the README needs a CVS tag.
Overall, apart from the points above, code is well-presented and uses Drupal's APIs.
Note that I've not searched for duplicated functionality.
Comment #5
siliconmeadow commentedHi Joachim,
Thanks for taking the time to review this - I'm glad you explained the context of the filter tips and the other points of style.
Regarding the filter kicking in after the line break filter, I hadn't considered that and I'm sure you're right. I need to examine that case and also see what happens with simpler regex if the line break filter isn't enabled and the content entry is completely reliant on a WYSIWYG editor which automatically wraps the return in <p></p>.
Do you think it differs in both cases, and if so, should I provide a contextual switch?
Comment #6
joachim commented> I need to examine that case
I think it's fine to say your filter MUST run before the linebreak filter, or in general, in a particular order relative to others. That's certainly what I do with one of my module's that's a filter.
Comment #7
siliconmeadow commentedIt looks to me that perhaps what I should be specifying is that the most likely use (but not exclusive) is on sites using a wysiwyg editor. If the linebreak filter is enabled, it filters multiple carriage returns and produces a similar result to this module.
If you have a wysiwyg editor enabled the linebreak filter does not act on a user's extra use of carriage returns, as it sees those lines as proper content - they have <p> tags.
Please help me in my gap of understanding.
Attached is a patch which adds the CVS tag to the README.txt file, standardises the naming of the module throughout (both sentence case and removal of camelCase) and makes the filter tips more user centric and friendly.
Cheers,
Richard
Comment #8
siliconmeadow commentedI just noticed that replies don't give you the option to change the status. Should have known, really, because that's the way comments work.
Here it is again. Needs review.
Comment #9
siliconmeadow commentedSorry - I borked the title earlier.
Comment #10
joachim commented> If the linebreak filter is enabled, it filters multiple carriage returns and produces a similar result to this module.
> If you have a wysiwyg editor enabled the linebreak filter does not act on a user's extra use of carriage returns, as it sees those lines as proper content - they have
tags.
Ohhh... I get it now (I'm all blocked up with cold at the moment, so a bit braindead.... )
So basically, this only applies to input formats that use a wysiwyg, because that causes a P each time you hit return. This could really do to be stated in the readme's introduction.
Hence this filter MUST work with the actual HTML and not line breaks, so that's all fine.
Comment #11
joachim commentedBTW, for CVS applications, please reupload the whole module rather than a patch :)
(Note to self or anyone else -- that's probably another item for the guidelines :)
Comment #12
siliconmeadow commentedI hadn't really considered that it would be something only useful with a wysiwyg input until now, tbh. I'll adjust README.txt accordingly and upload a new tarball.
Cheers!
Comment #13
siliconmeadow commentedNew version with yet more clarity, I hope.
Comment #14
IceCreamYou2 commentedLine 49, I'm pretty sure
<p>tags go outside of calls to t(). Other than that this is RTBC from me, especially considering the co-maintainer request; and I don't think that minor change is worth holding up this application.Comment #15
siliconmeadow commentedI thought that too, but after looking at the filter_tips in core's filter.module , I noticed it did it both ways (checked out line 180 or so). For shorter passages of text, I'm sure it makes it easier for the translators, so I've taken your advice and moved them out of t(). See the attached version.
Thanks for taking the time to review it!
Comment #16
drupalshrek commentedLooks great to me. Anybody who does a good README.txt is already halfway there in my book.
Comment #17
manarth commentedRTBC from me too.
Comment #18
avpadernoCan we get a review from joachim, who previously reported what needed to be changed (and did a good job)?
Comment #19
manarth commentedkiamlaluno: I've marked as RTBC because the co-maintainer request (in #2) meets the requirements based on http://drupal.org/cvs-application/requirements#comaintain (see #978746: Co-maintainer for the request and module maintainer's response).
I'm sure siliconmeadow will welcome any additional comments on the Empty Paragraph Killer module, but I don't think this should hold up the CVS application any further.
Comment #20
avpaderno