search

mintchaos / typogrify

A set of Django template filters to make caring about typography on the web a bit easier.
http://static.mintchaos.com/projects/typogrify/
Other
168 stars 29 forks source link

Fix widont newline behavior. #39

Open ryneeverett opened 10 years ago

ryneeverett commented 10 years ago

Fix #38.

Widont shouldn't count newlines as spaces. This is achieved with a regex inspired by this SO post. Rather than checking for a space (which includes newlines), we check for both (a) not not a space, and (b) not a newline.

chrisdrackett commented 10 years ago

Thanks for the fix! I don't have time to check this right away, but I'll try and get it tested and merged this week!

Kristinita commented 7 years ago

@mintchaos , @chrisdrackett , @justinmayer , please, review this pull request.

Thanks.

justinmayer commented 3 years ago

@ryneeverett: Please accept my sincere apologies for the absurdly long delay in reviewing your contribution. Truly.

In my cursory review of the changed output, it does indeed seem improved when there is a newline in-between the last two words. I found a case, however, in which the presence of two spaces were previously replaced with a  , but the new regex in this PR leaves those two spaces in place. For example:

“Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.”  —Socrates

Following is how Typogrify treats the two spaces between the closing quotation mark and the em-dash above:

That leaves me with two questions:

  1. Was this change intentional? I get the feeling that it isn't, given what the stated objective is.
  2. Is this change beneficial? I imagine not, but what do other folks think?
ryneeverett commented 3 years ago

I don't have much more insight into what I was thinking six years ago than you do. :) If that's indeed the effect then I'd agree this change is not beneficial. Feel free to close.