Add ability to trim at sentence break when using words

There seems to be a lot of overlap between this and ✨ Trim paragraph containing word count Closed: duplicate . It would be worth reviewing both as they both add a very similar option. This change seems to explicitly consider paragraphs in the splitting, which I don't think are part of the other.

Regarding the paragraph split, it seems to only use the <p> tag. I think it would be valid to include a list of paragraph divider tags, as headings, lists or other common types are not currently considered.

The change also seems to have sentence breaks hard coded. It would be good to use the sentence break characters constant defined 🐛 Error cropping Japanese Needs work , when that change gets merged.

re-rolled patch #15 to 2.1.x-dev

mjgruta → closed merge request !59

mjgruta → closed merge request !58

@mjgruta,

We appreciate your efforts on this issue, but I'm setting it back to "Needs work" for a couple of reasons:

1. There is no test coverage in the MR. All bug fixes and feature additions to the Smart Trim module require test coverage.
2. I don't think we have consensus on whether or not "sentence" should be a new trim unit (similar to "characters" and "words") or the functionality should be added via a separate checkbox (as the current MR does).

I'm not crazy about the logic of the current MR, as if the formatter is configured for a 50-character trim, but the sentence is 200 characters (not uncommon), then the trimmed text is going to be 4 times longer than the 50-character settings. I think this will lead to undesired results in a non-trivial number of situations. Therefore, I'd like to see this functionality implemented using "sentence" as a new trim unit. This would also have the added benefit of not adding a new checkbox to the formatter config UI.

-mike

Re-rolled 2815981-15.patch for 2.1.0

I can understand attaching a patch file might be useful for people to apply to live sites, but shouldn't the patch be taken from MR !60 rather than the previous #15 patch?

Updates from #30 break the patch. I have merged the latest code.

I just noticed the comment from mike. Reverting the status to Needs work.

I've made a few suggestions.

I think we also need some test cases that explicitly verify this.

Created a diff from commit 0779410f while fixing MR #60 to prevent the composer from failing to apply the patch.

Created another patch file from commit 975c9a47. This should fix the comments from James.

fixes $text_format type from null to string.

This patch fixes the fatals caused by the previous change.

@josh Sorry, the fix needs to be done on the field formatted viewElements as it is trying to pass the $item->format directly which will return null if the item doesn't have a format.
I have updated the code.

Fixed error in phpstan validation.

Hi @mjgruta, thanks; your latest change fixes the fatals I was getting with the previous one.

This fixes an error that popped up in our automated testing.

+1 to release this feature in the new module version.