TopShelfCraft / Wordsmith

A plugin for Craft CMS to help you manage and manipulate text.
Other
30 stars 20 forks source link

`widont` function ignores short words/neighbors #5

Closed wfendler closed 4 years ago

wfendler commented 6 years ago

Is there something I'm missing here? The   isn't being added at all to the second example below (and all other fields). I've looked through my plugins and I don't think there would be anything conflicting.

When I use the typogrify filter it works but I prefer not to use the "smart hyphens" but can't seem to find an example of "settings" in the documentation to turn this off.

{# This works #}
{% filter widont %}
  <p>Lorem ipsum dolor sit amet.</p>
{% endfilter %}

{# Doesn't work #}
{% filter widont %}
  <p>{{ work.headline }}</p>
{% endfilter %}

Thanks for your help!

michaelrog commented 6 years ago

I suspect it's an order-of-parsing issue. I've never run into this before, so I'll look into it and confirm. Meanwhile, may I recommend, instead:

  <p>{{ work.headline | widont }}</p>
jalendport commented 6 years ago

I've tested this on the latest versions of Craft and Wordsmith and couldn't replicate the issue.

Using the following in my template:

<p>{{ entry.wordsmithText }}</p>

{% filter widont %}
    <p>Lorem ipsum dolor sit amet.</p>
{% endfilter %}

{% filter widont %}
    <p>{{ entry.wordsmithText }}</p>
{% endfilter %}

displays this in the browser:

<p>Lorem ipsum dolor sit amet.</p>

<p>Lorem ipsum dolor sit&nbsp;amet.</p>

<p>Lorem ipsum dolor sit&nbsp;amet.</p>

as one would expect.

_It's highly possible this bug was fixed in Wordsmith (https://github.com/TopShelfCraft/Wordsmith/pull/4 for example) or in the Craft codebase during the period of time between when this issue was opened and when I tested it..._

Since it's working fine for me at this point in time I'm going to close the issue. However, if you seem to still be experiencing the issue, please leave a comment below and I'll definitely check further into the matter!

mattstein commented 5 years ago

FWIW, I chased this down and can confirm it's not a bug but an important implication of PHP-Typography's settings that apply to Dewidow_Fix. Most widow fixers I've used are just quick regular expressions, and this class will only handle widows in certain cases. Two settings matter in this case:

 * @param  int    $max_pull     Maximum number of characters pulled from previous line.
 * @param  int    $max_length   Maximum widow length.

max_pull and max_length both default to 5, meaning only widows fewer than five characters long will get a &nbsp; when the neighbor is also less than 5 characters long. So in this example...

{% filter widont -%}
    I end in lil words.
{%- endfilter %}

{% filter widont -%}
    I end in littleish words.
{%- endfilter %}

{% filter widont -%}
    I end with greater characters.
{%- endfilter %}

...only the first will get the non-breaking space:

I end in lil&nbsp;words.
I end in littleish words.
I end with greater characters.

If you choose more drastic settings in config/wordsmith.php...

return [
    'typographySettings' => [
        'set_max_dewidow_length' => 25,
        'set_max_dewidow_pull' => 25,
    ]
];

You'll get less picky widow-handling:

I end in lil&nbsp;words.
I end in littleish&nbsp;words.
I end with greater&nbsp;characters.
michaelrog commented 5 years ago

@mattstein, thanks for tracking this down.

I'm re-opening this issue and will look at setting some slightly more permissive defaults for max_pull and max_length.

(Changing these configs wouldn't technically be "breaking" from the standpoint of semver, but I wonder if it would be better to wait for a major release to change default behavior...?)

mattstein commented 5 years ago

A note in the docs or a static config example might be enough of a hint that nothing's broken. I kind of fancy it now.

I'd probably change the defaults with a major release since it could have visible consequences for live sites, but you're Mr. Manager @michaelrog!

michaelrog commented 5 years ago

A note in the docs or a static config example might be enough of a hint that nothing's broken. I kind of fancy it now.

Agreed.

(@jalendport, sending this one onto your docket.)

michaelrog commented 4 years ago

I increased the default threshold to 10 characters to make widont a bit more aggressive in closing up widowed lines. Released in 3.3.0. ✅

(Thanks again, @mattstein, for troubleshooting this for us!) 🎩