Use summary even when it is defined by <!--break--> via WYSIWYG editor

Thanks for this patch. I'm trying to decide if this is going to be expected behavior under most circumstances. I guess so, right? I mean I guess if I'm utilizing the functionality, I'd expect the trimmer to honor than and not output anything past that, right?

Probably a stupid question, but how does Drupal's built-in field formatter for "summary or trimmed" handle this?

Tried the patch and it's buggy because the text filters are not applied.

+++ smart_trim.module	2013-02-13 11:45:40.000000000 +0100
@@ -34,10 +34,15 @@
-        // option allows users to use the summary field IFF it is not empty.
+        // option allows users to use the summary field IF it is not empty.

IFF is intentional here and stands for if and only if.

+++ smart_trim.module	2013-02-13 11:45:40.000000000 +0100
@@ -34,10 +34,15 @@
+        if (!empty($settings['summary_handler']) && $settings['summary_handler'] != 'ignore' && (!empty($item['summary']) || strpos($item['value'],'<!--break-->') != 0)) {

You probably want to break out the || test here for clarity but it should work as intended.

In my opinion desired functionality should be use summary if present as it is authoritative, then fallback to "break" then to our auto trimmer.

The !=0 is unnecessary as the return value from strpos is false on no string result.

Attached patch respects the text filters and breaks out the logic a little more clearly. It also removes an unnecessary newline at the very end of the file because my vim configuration is an empty line trimming machine! ;).

Marking RTBC pending maintainer review as this is a contrib module and not core. Thank you for the great module!

This patch graciously sponsored by Second Melody.

+        $break_pos = strpos($item['value'], '<!--break-->');

broken out because its used more than once.

+        $has_summary_or_break = !empty($item['summary']) || $break_pos;

broken out for clarity as it is a conditional below that as logically grouped anyway

+            $output = check_markup(substr($item['value'], 0, $break_pos), $item['format'], $langcode, FALSE);

pulled in the $item['format'] dynamically from the item itself so it respects filters now.

Thanks @michaelfavia for the patch in #5

Applied and tested working..
Had to change the setting in display format to >> Use summary if present, honor trim settings (not the default setting)

I personally think this is the better default option where if user 'manually enters a teaser break' the system respects that manual override.

I'm also running into this issue and agree with the general sentiment of the thread that this module should respect the WYSIWYG teaser break .

One edge case note about the patch in #5 -- if the <!--break--> is the very first thing in the body, then strpos will return 0 (since it's in the zeroth position of the value string). In the line:

$has_summary_or_break = !empty($item['summary']) || $break_pos;

a $break_pos value of zero will evaluate as FALSE by php, resulting in the teaser break being ignored completely.

I'm not sure why someone would want a teaser break at the very start of their body (do they want to hide the entire body in teaser lists?), so I'm not arguing this is an important issue at all, I just wanted to point it out. If it does matter to anyone, using the not identical operator (!==) effectively distinguishes a zero-position integer value from a FALSE (not in string) value:

$has_summary_or_break = !empty($item['summary']) || $break_pos !== FALSE;

I found a flaw in the latest patch. This condition line does not treat breaks as summaries:

if ($settings['summary_handler'] != 'full' || empty($item['summary'])) {

I changed it to use the new variable defined in the previous patch

if ($honor_trim || $has_summary_or_break === FALSE) {

As you can see, I also added a new variable called $honor_trim. It does what you'd think (if trim is applied even on a summary/break), but I made it into a variable to make the code more readable.

As for the question whether this should be the expected behavior or not, I definitely think it should be, but I understand it wasn't in this module by default (just look at how extra much code was needed for basic support) :). I think the view function is getting bloated so maybe 2.x should break and abstract this summary/teaser thing into a function and then just treat whatever that returns as the summary? That way adding supporting for other possible summary concepts would be easier.

I implemented illeace's notes about $break_pos !== FALSE. I think it's a valid use case for someone to put a teaser break at the beginning of an article since that way you can ditch the teaser altogether.

So this is same as #9, but with $break_pos !== FALSE.

Just tried the patch in #10 and it works fine for me.

It works for me aswell when 'Use summary if present, honor trim' is set, but would it be possible to add a fix that shows the specified suffix (e.g: ...) in end even when <!--break--> is used. Now the suffix is NOT displayed when the teaser break is used.

Is this already implemented in the latest dev?
I think Smart Trim is an amazing module but i'm missing this feature tbh.

Patch #10 works for me =)

atiba: The patch is not in the dev version yet.

Apart from some offset, the #10 patch still applies cleanly also against the current dev. Works as expected. Please commit.

Thanks.

martin@martin-X501A1:~/smart_trim$ git apply -v smart_trim-fix-deny-trim-setting-with-teaser-break-1916060-10.patch
Checking patch smart_trim.module...
Hunk #1 succeeded at 45 (offset 5 lines).
Hunk #2 succeeded at 81 (offset 6 lines).
Applied patch smart_trim.module cleanly.

Re-rolled patch for current dev release. Would love to see this committed.

Re-rolled against DEV. Works great!

+1 for RTBC would love to see committed.

This seems to have been well-tested and useful over 2 years by various people, so worth adding to the module. Thanks!

Sorry I updated the codebase and this patch needs to be re-rolled.

also.. I am a moron who tried to patch this against the 8.x branch.. please hold.

Committed into the 7.x-1.x branch. Looking to do a full release this weekend. Please excuse my earlier silliness.

Thanks!

Great, many thanks

Hi all, can we please have a new stable release? The last stable is from 2015 while the commit was in 2016... we just lost a lot of time on this issue...

Committed into the 7.x-1.x branch. Looking to do a full release this weekend. Please excuse my earlier silliness.

Any news on this? :D