nl2br improvement

[Timbalu]

I know some Users don't like the convert ENTER newline(nl) to <br> function of the NL2BR Plugin (delivered by core).
Since Malte (onli) really improved the plugin with the p option most issues vanished, but still some did not match. It is a hard way coding all possible eventualities.

As I have been the (only?) one who is a apologist of still using the br option - knowing all these problems coming up when using handwritten or other markup plugins HTML in entries and independent from all further semantic questions - I added some enhancements to the plugin to discuss usibility here, before asking Garvin to upload.

My first (private) improvement (option clean_tags) tried to find a way to get rid of <br>'s done by the plugin in special cases like (table|thead|tbody|tfoot|th|tr|td|caption|colgroup|col|ol|ul|li|dl|dt|dd)
This did solve my private issues when writing handmade entries.

After listening to the last podcast n° 4, I wrote a comment with the rising idea of just using a simple and easy to remember tag, to shut down the NL2BR parsing inside an entry without getting complicated in coding all possible eventualities.

This is what I did now, uploading a version 2.1 here.

You can now just use <nl>do not parse newline to br inside</nl> to shut down the parsing multiple, but not nested, times in your entry.
I left the 'clean_tags' addition as a fallback option to use. But the ISOBR - as I called this default option - is working very nice and easy.

There still is the issue of not adding additional br or p tags, if the wysiwyg-editor, the textile, or markdown plugin already took care about markup. I tried to use this to prevent using NL2BR in these cases, but I am still not sure if this is bulletproof enough.

Code: Select all

                if ( $serendipity['wysiwyg'] || 
                              class_exists('serendipity_event_textile') || 
                              class_exists('serendipity_event_markdown') ) 
                {
                    return true;
                }

I changed and added some lang constants in <en>, <de> and <de-utf8>

serendipity_event_nl2br.zip

Update: added 4th version (2.11) on 2011-07-05
in special cases there was a leftover, which made editors with foldable Textblocks stumble

(20.87 KiB) Downloaded 346 times

[garvinhicking]

Hi!

Great, I'll have a look at this.

About your final comment; I don't think adding such a code snippet will do good in every case, so maybe you can make it as an option. For example, the 'wysiwyg' option only applies if a user is logged in; visitors will have wysiwyg not available. And about class_exists, it might be that people write blog entries where they usually disable textile, and only enable them for certain postings - and then the nl2br plugin would never work, also for those cases where people disabled textile conditionally...

Regards,
Garvin

[Timbalu]

Hi Garvin

Yes thats true. These kind of reasons I meant being not sure about the other markup shutdown. I think there are various possibilities possible, which can't be really checked clean & simple.

But do you really think an option check markup plugins true/false and disable/enable automatic markup check to disable nl2br plugin is the best way to avoid conflicts? (I hope this is the kind of option you meant.)

Maybe its just better to be clear in not supporting things like this and everybody using other markup plugins has to disable the NL2BR on their own via plugin administration?

EDIT:
Is there a variable always available, if textile or markdown are disabled temporarily? Does it depend which position in plugins queue they have?

Independently from mentioned further option

Code: Select all

if ( ($serendipity['wysiwyg'] && serendipity_userLoggedIn()) || class_exists('serendipity_event_textile') || class_exists('serendipity_event_markdown') ) {

[garvinhicking]

Hi!

What I meant was an option "Disable this NL2BR plugins, when WYSIWYG or specific markup plugins are detected" with true/false. The default being true, so that this could would work:

Code: Select all

                if ( serendipity_db_bool($this->get_config('disable_if_others') && ($serendipity['wysiwyg'] || 
                              class_exists('serendipity_event_textile') || 
                              class_exists('serendipity_event_markdown')) ) 
                {
                    return true;
                }

Is there a variable always available, if textile or markdown are disabled temporarily? Does it depend which position in plugins queue they have?

Yes, of course - other markup plugins check this option:

Code: Select all

...
if (                        !$eventData['properties']['ep_disable_markup_' . $this->instance] &&
                            !in_array($this->instance, (array)$serendipity['POST']['properties']['disable_markups']) &&
                            !$eventData['properties']['ep_no_nl2br'] &&
                            !isset($serendipity['POST']['properties']['ep_no_nl2br'])) {
 // yes, this markup shall be applied
} else {
 // no, do not apply markup
}

$eventData comes from entry_display hook, so it's per entry.

HTH,
Garvin

[Timbalu]

yes just a minute I'll try to resume, the option I did yesterday...

[Timbalu]

Hi Garvin

I expect I dont need the $eventData['properties']['ep_no_nl2br'] in here, but are there $eventData['properties']['ep_no_textile'] and $eventData['properties']['ep_no_markdown'] available?

[garvinhicking]

Hi!

Yes, the no_XXX part shoudl correlate with XXX = serendipity_event_XXX.

Regards,
Garvin

[Timbalu]

(letzte) Frage:
Das nl2br läuft ja im Wesentlichen im 'frontend_display' hook.
Der hook 'entry_display' wird erst danach aufgerufen, oder?

Kann ich denn auf diese 'ep_no_XXX' auch im 'frontend_display' hook zugreifen, oder muss ich jetzt alles in den 'entry_display' hook umschreiben? [Dein Beispiel code setzt bei mir jetzt im 'entry_display' eine Serendipity variable true/false, auf die ich gerne im 'frontend_display' zugreifen würde. Verständlich?]

[garvinhicking]

Hi!

I think so, I mixed up entry_display and frontend_display. Use frontend_display, this is what I meant.

Regards,
Garvin

[Timbalu]

No no, I dont' have eventData available in frontend_display. I think I have to move everything to entry _display.

At least I can't print_r($eventData) in frontend_display, which is funny, since manipulating $eventData elements work as expected. So, as I don't have test cases with textile or markdown, I tried to debug with print_r().
Btw, the nl2br works good in both cases: everything in frontend_display, or everything in entry_display.
I am not sure which is better to use by performance.

EDIT:
Sorry - the test blog I tried with, had only entries cached by the entyproperties plugin, I think this is why I didn't get any output from eventData. I'll use frontend_display now.

[Timbalu]

Ok I finalized it and updated the zip in my first post.
Please test, comment, or add to SVN.

Maybe we can move this thread to plugin or development section...

[garvinhicking]

Hi!

Ok! I'll supossedly not have time to check it out before tuesday. If there are others who want to comment on this, please feel free.

Regards,
Garvin

[Timbalu]

@Garvin
A Thuesdays reminder!

[garvinhicking]

Hi!

I only found time to do a quicklook - it seem syou use an older nl2br as template. The current one with s9y 1.6 ships with a different header. Can you merge that patch?

Other than that, the patch looks good to me. Are there more people maybe who could try this updated plugin in their environment, if they use nl2br? It seems a bit complex, so the more users could test this, the better - even though from reading the code I believe it shoud work flawless.

Regards,
Garvin

[Timbalu]

Well its only this differing in the header....

Code: Select all

@serendipity_plugin_api::load_language(dirname(__FILE__));

which will not work with Serendipity versions < 1.6, right?
Thats why I didn't add it ... (beside of the point I didn't remember)