Quoted-printable encoding unrecognized on GitHub #15
Comments
|
@mariozaizar Did you get a chance to look into this? |
|
@pchaigno I'm sorry, somehow I'm didn't got this notification until now. Let me take a closer look in the next couple days. Thanks for reporting it 🙏 |
|
|
@wesinator So the highlighting is the same as with Lightshow? Could you post a screenshot of the result? |
|
@pchaigno No, Atom has the expected behaviour. is not highlighted red.
|
|
@wesinator Ah, I see. Thanks! @Alhadis Looks like there is a discrepancy between the interpretations GitHub and Atom are having for this grammar. I couldn't find any non-PCRE-compliant regular expressions, but maybe you will...? |
|
The expressions aren't the problem. This is: # Content-Type: text/html;
{
'match': '^(?i)<html>(.|\s)*<\/html>$',
'include': 'text.html.basic'
},We're still using the old TextMate grammar for HTML highlighting, which is the likely culprit. Ergo, we have two choices:
|
|
FYI, this grammar has numerous mistakes (several patterns are incorrectly escaped, and the HTML pattern mentioned above is ignoring the # Content-Type: text/html;
{
'match': '^(?i)<html>(.|\s)*<\/html>$',
- 'include': 'text.html.basic'
+ 'captures':
+ '0':
+ 'patterns': [
+ {
+ 'include': 'text.html.basic'
+ }
+ ]
+ }
},Or much more succinctly: # Content-Type: text/html;
{
'match': '^(?i)<html>(.|\s)*<\/html>$',
- 'include': 'text.html.basic'
+ 'captures':
+ '0': 'patterns': ['include': 'text.html.basic']
},But this alone is inadequate, because:
The only reason there even is HTML highlighting is because of the incorrect I'll send a pull-request. |
|
Sorry folks, I'm quite disconnected from work and opensource for the last weeks due to an imminent baby arrival 🍼 ; but if you guys want to open PRs against this addressable comments I'm more than happy to review and merge to keep things moving. Thanks again! |
|
💡I just published John's fixes #16 as v1.1.1. Unfortunately, I'm still seeing the red background on Lightshow using the new version in the same way as the old version, but as Ԝеѕ pointed out, Atom doesn't show any red background 🤔
|
|
Aye, that's because it's loading the old TextMate grammar, which is applying the highlighting. I'm literally in the middle of prepping a PR to get it replaced on Linguist's side. Once merged, changes will become visible across the site when the next release of Linguist has been cut. |
|
Closing this as it's an issue with the HTML grammar. Thanks a lot @Alhadis! @mariozaizar Once the fix is in place at Linguist, we're likely to use this grammar to highlight all |
Prerequisites
I haven't reproduced the issue in Atom. Please see the description.
Description
We're considering (cf. github-linguist/linguist#4201) using this EML grammar to highlight all
.emland.mboxfiles on GitHub and GitLab. Unfortunately, the grammar's output has some errors (see Lightshow example) for quoted-printable encoding (if that's not obvious, I don't know much about EML).I haven't tested under Atom yet, so it's possible the issue is only with GitHub's interpreter for grammars. GitHub uses a PCRE-based regular expression engine, which has a few differences in interpretation. I had a quick look at the grammar and did not spotted any though.
Steps to Reproduce
Expected behavior: Characters following the quoted-printable marker (
=3D) to be highlighted correctly.Actual behavior: Characters following the quoted-printable marker are highlighted with a red background.
Reproduces how often: 100%
Versions
The highlighting bug happens with commit c9f354e of the present grammar.
Additional Information
Lightshow is a simple interface to test the highlighter used on GitHub.com. It simply takes the grammar provided and interprets it on the given example.
The text was updated successfully, but these errors were encountered: