fix: crash in GFMAlerts when falling back to blockquote #50
Conversation
| { | ||
| $this->BlockTypes['>'][] = 'Alert'; | ||
| } | ||
| private $icon = array( |
There was a problem hiding this comment.
I moved the icons into a private property so that they weren't being created again each call to blockQuote()
|
|
||
| public function __construct() | ||
| { | ||
| $this->BlockTypes['>'][] = 'Alert'; |
There was a problem hiding this comment.
This was overwriting the default blockQuote definition, unnecessarily, requiring a blockAlert function. Instead, we just override blockQuote() and add handling when an alert pattern is found.
| if(!preg_match('/^\>\s*\[\!(IMPORTANT|TIP|NOTE|WARNING|CAUTION)\]/i', $line['text'], $matches)) { | ||
| return parent::blockQuote($line); | ||
| } | ||
| } |
There was a problem hiding this comment.
If the string is not an alert pattern, then just fall back to the default blockQuote handler
| // We set isAlert to allow subsequent lines to be part of the alert | ||
| 'isAlert' => true, |
There was a problem hiding this comment.
This is what was needed to allow us to track alert styling for blockQuoteContinue calls
| if(!preg_match('/^\>\s(.*)/', $line['text'], $matches)) { | ||
| return null; | ||
| return; |
There was a problem hiding this comment.
It isn't clear to me if we should be return null or just return, because both seem to be used in Parsedown. However, blockQuoteContinue parent handler just does a return for these kinds of cases, so copied that
| return $block; | ||
| } | ||
|
|
||
| protected function blockQuoteComplete($block) { |
There was a problem hiding this comment.
This function was not needed, because it did nothing
The code paths when a normal blockquote was encountered were not quite right. Also tidied up a few other bits and pieces for robustness. Fixes: #49 Fixes: HELP-KEYMAN-COM-DH8
mcdurdin
force-pushed
the
fix/49-alerts-crashing-with-blockquote
branch
from
3d97cf1 to
9a20ed4
Compare
|
I'm doing a test on localhost at the moment. I will be back with more info. |
|
LGTM 😄 A minor thing I found, @mcdurdin. I'm using two if(preg_match('/^\>\s(.*)/', $line['text'], $matches)) {
if(!isset($block['isAlert'])) {
return;
}
}Test: > [!IMPORTANT]
> This is important.
> Another important line.
> [!NOTE]
> Keyman Facts.
> [!TIP]
> Keyboard layouts are defined with a clear and easy to understand keyboard grammar.
> [!CAUTION]
> Keyboard layouts are distributed through an open catalog to all major desktop and mobile platforms.
Continue your Markdown documentation.
> [!WARNING]
> This is the last GFMAlerts.
> [!TIP]
> [!IMPORTANT]
Paragraph
# Heading
* This is the last GFMAlerts
> This one won't show // if two `>` are together like this, the top one won't show?
> ParagraphResult: |
| } | ||
|
|
||
| // Handles any interruption | ||
| if(isset($block['interrupted'])) { | ||
| return $block; | ||
| return; |
There was a problem hiding this comment.
Good cleanup.
I didn't see where 'interrupted' ever gets set
There was a problem hiding this comment.
This is a clone of the Parsedown code -- interrupted is set by Parsedown
Here's what I am seeing (top is cropped off):
|
|
and this: gives this:
|
The code paths when a normal blockquote was encountered were not quite right. Also tidied up a few other bits and pieces for robustness.
Fixes: #49
Fixes: HELP-KEYMAN-COM-DH8