- 🇺🇸United States ultimike Florida, USA
I like this idea.
Thanks to everyone for their help so far on this task (especially the test coverage!)
I went ahead and added the "Trim sentence" stuff to the settings summary.
Patch and interdiff attached.
Needs a solid review before committing.
thanks!
-mike
- Status changed to Needs work
over 1 year ago 12:28am 19 April 2023 - 🇺🇸United States markie Albuquerque, NM
Re-roll needed after previous commit.
- 🇺🇸United States ultimike Florida, USA
I re-rolled and issue-branched the previous patch.
The more I think about this, the more I wonder if this functionality should be implemented as a new option for "Trim units" (adding a new "sentences" option) instead of the current "Trim sentences" checkbox.
I can't think of any major pros/cons of either approach, but I do think adding a new "sentences" option for "Trim units" does seem a tad bit more elegant than the "Trim sentences" checkbox included in this task.
Thoughts?
-mike - last update
over 1 year ago 20 pass - @markie opened merge request.
- 🇺🇸United States markie Albuquerque, NM
I tested out the branch and it is working, but I also agree with Mike. I am torn on using "sentences" as the trim unit or maybe "words (to next hard stop)" or something like that. That being said.. lets move the hidden checkmark into the trim unit options.
- 🇮🇪Ireland lostcarpark
The change in MR !41 looks good, but why only offer the option when trimming by word? I think it would be reasonable to have the option to limit the output to 500 characters, trimmed at a sentence break.
Also, I would encourage moving the sentence dividers to make it easier to add additional break characters.
- 🇮🇪Ireland lostcarpark
Regarding my last point, the patch in 🐛 Error cropping Japanese Needs work comment #33 already defines a constant containing sentence break characters.
- 🇮🇪Ireland lostcarpark
This change seems to have a lot of overlap with ✨ Add ability to trim at sentence break when using words Needs work . It would be worth reviewing both and comparing. Both seem to add a very similar option to the settings.
- Status changed to Closed: duplicate
over 1 year ago 3:05pm 7 June 2023 - 🇺🇸United States markie Albuquerque, NM
After discussing, we have decided this is very similar to https://www.drupal.org/project/smart_trim/issues/2815981 ✨ Add ability to trim at sentence break when using words Needs work which will add a new trim unit of "sentence". See the above issue for more information. Thanks for your feedback