Code snippets: text in angle brackets become lowercase

[damithc]

For example <Person> becomes <person> after rendering to html.
The current workaround is to add extra spaces < Person > which stays as < Person > after rendering.

[damithc]

It is also causing problems for operators such as <=

Seems to cause problems even in normal text

x <= y
x <= y

[nicholaschuayunzhi]

I would like to try this issue but am having problems with reproducing the bug.

I tried this

```
<Person>
```

But it renders as

[acjh]

The example in the description can be found here:
Admin > v1.5rc [week 12] > Tips: how to score high for code quality > code quality guidelines covered in the module > Guideline: Name Well > Basic > Use Nouns for Things and Verbs for Actions

ArrayList<Person> students becomes ArrayList<person> students

[damithc]

Sometimes it can even break the whole page. It seems angle brackets inside code snippets are being interpreted as instructions to the parser rather than being ignored as plain text.

Another example where the problem occurs if I use <words> instead of < words >

>>> type(True)
< class 'bool' >

>>> type(False)
< class 'bool' >

>>> type('True')
< class 'str' >

[danielbrzn]

It seems like this is a problem with the htmlparser2 library that Markbind is using. This is in fb55/htmlparser2#220 and it does not seem that it will be fixed.

Should we consider changing to another HTML parser library like Parse5 or should we fork htmlparser2 and try to fix it ourselves?

[Gisonrg]

I think htmlparser2 is not actively maintained and buggy as well...
However changing to another HTML parser library could easily worth a few weeks' efforts and basically a re-rewrite of the parser. Let's create an issue to track this.

[danielbrzn]

I managed to fix it by modifying the behaviour of htmlparser2 for Markdown code, but I suppose it would require a fork of htmlparser2 with our own modifications. What does everyone think?

[damithc]

Nice work @nicholaschuayunzhi .
It depends on how extensive the patch is and how reliable the fix is.
It also depends on how active their repo is. If it is active, we can submit a PR and get it merged so that we can discard our own fork and go back to using the upstream version at some point in future.
On the other hand if the repo is dead, we can use our own fork for the time being (with your fix) but for the longer term we might have to find a replacement library anyway.

[nicholaschuayunzhi]

@damithc , I think you should credit @danielbrzn, he did the work.

[acjh]

Creating a fork for this fix sounds reasonable, since the repo is dead.
This means that MarkBind will need to publish our fork on npm, right?

This is a case where a test in #121 would be necessary, for when we change the parser in the future.
As discussed, that change is on hold — considering that cheerio depends on htmlparser2 as well.

[damithc]

@damithc , I think you should credit @danielbrzn, he did the work.

Yup, sorry.

[danielbrzn]

@damithc
It's a very small fix that forces the parser to treat markdown code like x<y as text instead of having it parse the information in between the quotes. This allows the markdown code to remain as is until we call the markdown renderer to properly render it to HTML. When testing using the live site, it seems that everything renders properly but there was a small change that I had to make to website\admin\appendixB-polices.md as there was an extra ' ` ' character at the end of the stackoverflow link under the section "giving credit for reused work". This extra character was causing the fixed parser to not be able to properly resolve the base URLs.

@acjh
Actually, it seems that the developer has finally returned. However, he did mention to use parse5 if a spec-compliant parser was necessary.

I feel that the fix that I have is quite specific to our project and I'm not sure if it would be appropriate to submit it as a PR to the upstream version.

[acjh]

@danielbrzn Noted. But isn't it specific to Markdown-in-HTML, rather than MarkBind?

This means that MarkBind will need to publish our fork on npm, right?

@Gisonrg Shall I set up a repo for this?

[danielbrzn]

@acjh Ah yes, that's true. I guess we could consider making a PR to the upstream repo.

About publishing our fork on npm, how about we have it as a private module? We can still have the fork, but I don't think it's necessary to publish it on npm.

[acjh]

Ah yes, a private module.

[damithc]

@danielbrzn
See the following part in https://nus-te3201.github.io/website/programming/index.html#program-flow-control

I had to do this ugly workaround because of this issue. See if your fix works for this code too.
The code is at https://github.com/nus-te3201/website

Specifically, https://github.com/nus-te3201/website/blob/master/programming/booleans/text.md

[danielbrzn]

@damithc

Seems like it works!

[damithc]

Seems like it works!

Nice.

[acjh]

Forked to MarkBind/htmlparser2