Add whitespace control to LIQUID_TAG_REGEX

[kylebarbour]

Fixes #7009. Simply adds -? to the beginning and end of the regexes used in /lib/jekyll/excerpt.rb to allow for Liquid whitespace control characters.

[pathawks]

Oh sweet! Thanks for catching this @kylebarbour 👍

@ashmaroli Does LIQUID_TAG_REGEX have any tests? Does this PR need to add any, or maybe we should make a note to tackle that later?

[ashmaroli]

@kylebarbour Thank you for opening the PR.
The regex changes look good. This does need a test too though.

You may refer to #6724 to see what tests are currently in place to test this functionality. If it looks confusing to you, the following note should guide you:

All you need to do is (at minimum):

• add two new posts in the test/source/_posts directory, each containing Liquid Block-type tag with the whitespace_control character.
  One post (test subject) will contain only the "opening" construct as part of the excerpt. (The "closing" construct is to be written after a blank line). The other post (control subject) will contain the same content but without the blank line before the "closing" construct.
• append assertions to test/test_excerpt.rb using the above two posts.
• adjust test/test_generated_site.rb.

Run script/test test/test_excerpt.rb to see if your patch passes the base test.

(Optional): Once you're comfortable with the tests, you can add and test more posts with various valid placement of the whitespace-control character.

If you still find it hard to add tests for this PR, feel free to inform so here.

[kylebarbour]

Thanks for guiding me through the process! I'll take a stab at it.

[kylebarbour]

@ashmaroli Take a look and see what you think — I think this all works!

[kylebarbour]

Added a more informative error message when something resembling a Liquid tag is found but doesn't have superclass Liquid::Block.

[ashmaroli]

@kylebarbour The tests look good. 👍

The message to Jekyll Logger converts all "whitespace" to a "single space" character. Therefore, Jekyll.logger.error "Error:", "foo\nbar" will simply output Error: foo bar..

So, IMO this should be in 2 separate calls:

Jekyll.logger.error "Error:",
  "This page's excerpt appears to contain a Liquid tag which couldn't be parsed"
Jekyll.logger.error "", e.message

Disclaimer: The suggestion above has not been tested locally..

Since, I can't test locally, I'm not sure if Jekyll has to log e.message at all, since we're going to be calling raise with the same message..

[kylebarbour]

Great! You're right, logging e.message is redundant, so I've removed it.

[pathawks]

This seems like a nice compromise to me for something like a 3.8.2: a much more helpful error message without any material change in behavior 👍

[ashmaroli]

Can we shorten the message a bit..? :

Jekyll.logger.error "Error:",
  "Liquid tag in #{doc.relative_path} couldn't be parsed."

[pathawks]

Should mention “excerpt” to hopefully narrow things down for the user.

[ashmaroli]

@pathawks "excerpt" will be in the path because the error arises only for Excerpt instances. For example:

Liquid tag in _members/john-doe.md/#excerpt couldn't be parsed.

[pathawks]

I would favor a more descriptive error message over a less descriptive one, but if you think this is sufficient, 👍

[kylebarbour]

I've written a slightly more succinct error message — what do you think?

@ashmaroli I don't think the word excerpt will necessarily be in the path, as it wasn't in my original error that mentioned the superclass (see my comment in #7009). The excerpts seem to be generated for everything whether they're needed or not; I get about 20 ms of performance improvement from commenting out generate_excerpt in lib/jekyll/document.rb.

Running jekyll build on my test case without the custom error message, all that I get is

Error: could not read file <file>: undefined method `superclass' for nil:NilClass

without anything about excerpts.

[ashmaroli]

This is the reason why we decided to add a helpful error message.. 😉

[pathawks]

This makes a lot of sense to me 👍

[ashmaroli]

Lets not have a trivial log message block this.. All changes LGTM!

[pathawks]

@jekyllbot: merge +fix

[kylebarbour]

Thanks all! You were really helpful and fun to work with — as a truly rookie programmer I appreciate it.

[pathawks]

@kylebarbour Thanks so much for all your work on this. Hoping this won't be your last contribution 😉🍻

[ashmaroli]

@pathawks Would you mind handling the backport of this patch for a v3.8.2..?