Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

highlightAuto with language subset doesn't appear to be working #1868

Closed
Martii opened this issue Oct 14, 2018 · 7 comments
Closed

highlightAuto with language subset doesn't appear to be working #1868

Martii opened this issue Oct 14, 2018 · 7 comments

Comments

@Martii
Copy link
Contributor

Martii commented Oct 14, 2018

Hello,

Re:

Spec:

$ node -v
v10.12.0

NOTES:

  • https://openuserjs.org/…#comment-16627b793f6 (fixed this one to be a named fence to ensure correct library is being loaded and it is along with the CSS we inject... so it is currently working with named ones just not automatic detection).
  • All languages are available for named fences however we have had to limit unnamed fences since auto-detection doesn't always pin the right language. Been through a few revisions and landed on this methodology as the best available for our use cases.
  • Same issue in development. Clean package installs, etc.
  • We use ES5 syntax as a default and minimal to none ES6+.

Thanks for the look.

@egor-rogov
Copy link
Collaborator

Hi @Martii
The problem is in xpath. There is no such language in highlightjs:

  highlight: function (aCode, aLang) {
    var obj = null;
    var lang = [ // NOTE: More likely to less likely
      'javascript', 'xpath', 'xml',         // <------- remove xpath
        'css', 'less', 'scss',
          'json',
            'diff',
              'shell',
                'bash', 'dos',
                  'vbscript'
];

In 9.12 this fact was silently ignored, but now we throw an error. Probably we should not.
So, quick fix for you is to remove xpath from the list of languages.

@Martii
Copy link
Contributor Author

Martii commented Oct 14, 2018

@egor-rogov

The problem is in xpath. There is no such language in highlightjs

That did the trick. Will push to dev. Thank you! Could have sworn that I validated all the language names but I'm only human. ;) :)

In 9.12 this fact was silently ignored, but now we throw an error.

Looks like I'll need to add a few more dev console messages to track this. :) Good to know.

Thanks a bazillion!

@Martii Martii closed this as completed Oct 14, 2018
@Martii
Copy link
Contributor Author

Martii commented Oct 14, 2018

Hmm just trapped on local dev and nothing thrown. :\ Oh well... I'm ready for rest and will tackle our issue later.

@Martii
Copy link
Contributor Author

Martii commented Oct 14, 2018

Okay I'm stubborn... trapped every layer of our code, and several hard refreshes of my browser, and got this:

TypeError: Cannot read property 'disableAutodetect' of undefined
    at autoDetection (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/highlight.js/lib/highlight.js:711:29)
    at Array.filter (<anonymous>)
    at Object.highlightAuto (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/highlight.js/lib/highlight.js:572:40)
    at Object.highlight (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/libs/markdown.js:261:21)
    at Renderer.code (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/marked/lib/marked.js:920:28)
    at Parser.tok (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/marked/lib/marked.js:1199:28)
    at Parser.parse (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/marked/lib/marked.js:1144:17)
    at Function.Parser.parse (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/marked/lib/marked.js:1126:17)
    at marked (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/marked/lib/marked.js:1533:19)
    at exports.renderMd (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/libs/markdown.js:285:10)
    at exports.renderComment (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/libs/modelParser.js:719:30)
    at Function._.map._.collect (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/underscore/underscore.js:205:24)
    at preRender (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/controllers/issue.js:240:13)
    at asyncComplete (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/controllers/issue.js:254:11)
    at ~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/async/dist/async.js:3888:9
    at ~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/async/dist/async.js:473:16

... that's not quite the stack trace I was thinking I was going to get... disableAutodetect of undefined.


Anyhow... removing the xpath seems to do the trick for now.

@Martii
Copy link
Contributor Author

Martii commented Oct 14, 2018

The problem is in xpath. There is no such language in highlightjs

There should be... it's listed at the tail end of https://github.com/highlightjs/highlight.js/blob/82c22c6da9f591510f041d2047c0a5b7e9886370/docs/css-classes-reference.rst#language-names-and-aliases.

See XQuery | xpath, xq.

I'm going to reopen this as this is darn peculiar.

@Martii Martii reopened this Oct 14, 2018
@egor-rogov
Copy link
Collaborator

Ah, I see. It doesn't accept aliases (and never did), so use xquery instead.
Please leave the issue open, we'll need to rethink this. Probably we should accept aliases as well.

Martii added a commit to Martii/OpenUserJS.org that referenced this issue Oct 14, 2018
Martii added a commit to OpenUserJS/OpenUserJS.org that referenced this issue Oct 14, 2018
* Add in some dev console messages to assist

Post #1528 #1438 ; Applies to #430 ; See also highlightjs/highlight.js#1868

Auto-merge
@egor-rogov
Copy link
Collaborator

egor-rogov commented Oct 15, 2018

Okay, basically we have two options here:

1. Update the docs to reflect the current behavior (i. e. that highlightAuto() and configure() don't accept aliases). Previously aliases were accepted but ignored (that's too bad), now an error occurs. This change appeared quite accidentally, it wasn't planned.
2. Tweak the code to actually accept aliases along with language names.

(2) is doable but seems not worth the effort. It shouldn't be a big problem for the caller to specify a right name. So I'm inclined to (1).
@marcoscaceres what do you think?

I'm wrong, sorry for the buzz. That's my bug :( Will fix it.

egor-rogov added a commit to egor-rogov/highlight.js that referenced this issue Oct 16, 2018
Martii added a commit to Martii/highlight.js that referenced this issue Oct 16, 2018
Add default non-aliased language as the first item for XQuery to match the rest of the doc. e.g. `xquery`

Post highlightjs#1868
marcoscaceres pushed a commit that referenced this issue Oct 17, 2018
Add default non-aliased language as the first item for XQuery to match the rest of the doc. e.g. `xquery`

Post #1868
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Oct 28, 2018
* Please read their CHANGELOGs
* Revert back to alias from OpenUserJS#1529 for Code readability now that it is fixed; See also highlightjs/highlight.js#1868
* Delete op retested
Martii added a commit to OpenUserJS/OpenUserJS.org that referenced this issue Oct 28, 2018
* Please read their CHANGELOGs
* Revert back to alias from #1529 for Code readability now that it is fixed; See also highlightjs/highlight.js#1868
* Delete op retested

Auto-merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants