Markdown glitch on Docs module?

[RuiNtD]

This is being printed on the tut_blog page of the docs (at least on x10hosting):

Create a new file in your new config folder, called config.php.

'Blog',
        'description'   => 'A Simple Blog Example',
        'author'        => 'Your Name',
        'homepage'      => 'http://...',
        'version'       => '1.0.1',
    'menus'       => array(
        'context' => 'path/to/view',
        ),
        'weights'       => array(
        'context' => 0,
    ),
);

Not all of these settings will be used for the Blog module, but you should understand how they work.

Name is the human-readable name of your module.

Description is a short description of your module and will appear in a list of installed modules.

[ . . . ]

That tag goes on till the end of the document. When I check the source code, it shows:

Create a new file in your new config folder, called config.php.

 'Blog',
[ . . . ]
```

Firefox detects everything from `` as a XML processing instruction, which isn't supported in HTML, so Firefox ignores it.            

[RuiNtD]

I just noticed:

ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): Entity: line 437: parser error : Opening and ending tag mismatch: div line 436 and p /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): <h3>New Post</h3></p> /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): ^ /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): Entity: line 641: parser error : Opening and ending tag mismatch: p line 435 and div /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): r Bonfire to get you well on your way to creating the next killer app.</p></div> /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): ^ /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): Entity: line 641: parser error : Premature end of data in tag div line 1 /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): r Bonfire to get you well on your way to creating the next killer app.</p></div> /home/********/bonfire/modules/docs/controllers/docs.php 364
ERROR - 2015-03-07 19:06:16 --> Severity: Warning --> SimpleXMLElement::__construct(): ^ /home/********/bonfire/modules/docs/controllers/docs.php 364

[mwhitneysdsu]

Is this with the released/master version (0.7.1) or with the version in the develop branch? I'm not seeing the issue locally, so I think it may have been fixed in 223e8084887b8f795cda57986ad68dd898b94e91

[RuiNtD]

Released version. It probably WAS fixed in that commit, but the bug in the Markdown parser would still exist, which would not be good for anyone wanting to use it. Maybe try updating the parser? Also, you may want to try using the recommended PHP parser linked on the Markdown website.

[mwhitneysdsu]

While I'm not closing the issue, I'm not sure how long this will take unless someone submits the required changes. The docs use Markdown Extra Extended, and both the Markdown Extra and Markdown Extra Extended libraries are currently implemented as helpers.

The Markdown Extra parser could probably be updated to 1.2.8 (from the current 1.2.7) with a little less effort, but I'm unsure whether this would change that particular issue until I do some testing (and, in the long term, updating Markdown Extra to 1.2.8 is probably a waste of time, given that it was released in Nov. 2013).

Since it supports GFM, it may be worth looking at Parsedown

[lonnieezell]

The errors are SimpleXML-related from what you've shown, not Markdown parsing. While I don't have a chance to look into just yet, I have seen issues where it was getting an extra div that had to be cleaned (or the other way around? Not sure been awhile).

This does seem to be potentially serve specific since we're not seeing it and this is the first report in 4 years about that specific error.

I will try to dig into it a little more this evening, though, and see if I can reproduce it at all.

[mwhitneysdsu]

@lonnieezell You should be able to replicate the error by reverting the tut_blog.md file to the version prior to the changeset I linked above. Or you can just modify the code block cited by surrounding it with three backticks before/after the block and removing the indentation. It may also be related to the fact that the version of the Markdown Extra parser is not the one that shipped with the Markdown Extra Extended parser, because I upgraded it at some point.

[lonnieezell]

Ah - so it definitely is a Markdown parser thing by the sounds of it, then.

Might be worth looking into the new CommonMark libraries. Looks like the League has one.

[lonnieezell]

Damn - they have a PHP 5.4 requirement which doesn't seem like the right fit for Bonfire ATM, though that will be the minimum for Bonfire Next.

[mwhitneysdsu]

Parsedown claims to pass most of the CommonMark tests and run on PHP 5.2+.

After looking at the generated code, the issue is clear in the original post. The tut_blog.md file used to look like this: (it looks like GFM is stripping the PHP tags...)

Create a new file in your new `config` folder, called `config.php`.
```
 'Blog',
    'description' => 'A Simple Blog Example',
    'author'      => 'Your Name',
    'homepage'    => 'http://...',
    'version'     => '1.0.1',
);
```

which generated something like this:

<p>Create a new file in your new <code>config</code> folder, called <code>config.php</code>.</p>

<pre><code><?php defined('BASEPATH') || exit('No direct script access allowed');

The updated file looks like this:

Create a new file in your new `config` folder, called `config.php`.
     'Blog',
        'description' => 'A Simple Blog Example',
        'author'      => 'Your Name',
        'homepage'    => 'http://...',
        'version'     => '1.0.1',
    );

which generates something like this:

<p>Create a new file in your new <code>config</code> folder, called <code>config.php</code>.</p>

<pre><code>&lt;?php defined('BASEPATH') || exit('No direct script access allowed');

The SimpleXML parser chokes on the <?php tag because it can't find a matching closing tag, but when the Markdown parser converts it to <?php, there's no problem.

[lonnieezell]

So the current Bonfire is already fixed and doesn't generate this code currently? Good deal.

Switching to Parsedown sounds like a good plan.

[mwhitneysdsu]

The document is fixed, but the parser will still generate broken code if you use the GFM-style code blocks with an opening PHP tag in the code block.

[mwhitneysdsu]

I implemented Parsedown locally, but immediately ran into an issue when I went to update the changelog. Parsedown incorrectly interprets a sequence like #793 as a heading, which makes the changelog's list of closed issues in each version look bad. The alternatives while keeping Parsedown would be either updating all of the entries in the changelog or changing the CSS, either of which would be hiding the issue, just like changing the docs for the tutorial hid the original issue reported here.

[mwhitneysdsu]

As mentioned in the commit above, I've changed the way the docs module interacts with the parser to make it easier to replace the parser (and make the parser configurable). I'm writing the documentation for the new library and looking into any additional functionality which may need to be added. I put the configuration options in application/config.php because I wanted the CommonMark library to be separate from the docs module, though I may add a separate config option in the docs module to allow the docs module to use a different configuration of the library from the application-level configuration, if the user desires.

I added the Parsedown library and its driver for our CommonMark library to /application/libraries/ as an example implementation. As part of my documentation effort, I intend to implement a driver for League\CommonMark\CommonMarkConverter, but I don't intend to include it in the Bonfire repository due to it requiring a higher version of PHP than Bonfire's current baseline.

While I was hoping the Parsedown library would fix this issue, my previous comment should indicate why that is not the case. I'm hoping that I can find a library which works with Bonfire's current minimum PHP version requirements and requires no (or minimal) modification of the existing docs. In the long term, I'd like to ensure that the docs conform to the CommonMark spec, so they will work with (hopefully) as many parsers as possible in the future.

[mwhitneysdsu]

OK, I believe I'm done with this, for now. The only reason I'm not closing this is because the default driver still exhibits this behavior, and the best drivers currently available will require installing another library (via Composer or manually). I've included drivers (in /application/libraries/CommonMark/drivers/) for:

• Parsedown v1.5.1 - library included in /application/libraries/Parsedown.php
• Markdown and MarkdownExtra v1.5
• The PHP League's CommonMark v0.7.2

The PHP League's parser has given me the fewest issues overall, but, as mentioned earlier, requires a higher version of PHP than Bonfire itself. So, if you're running Bonfire on PHP 5.4.8+ (as I am) and you have Composer installed, you should probably add "league/commonmark": "0.7.2" to your composer.json and set the following in your /application/config/application.php: $config['commonmark.driver'] = 'LeagueCommonMark'; (just make sure LeagueCommonMark is in your commonmark.valid_drivers setting, too).

I've included docs for the library, including some notes on building drivers. The driver can be configured separately for the docs module and the application by setting docs.commonmark_driver to the name of the desired driver in /bonfire/modules/docs/config/docs.php (I included the setting in a comment at the end of the file).