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

lisp highlighting very bad #1533

Closed
dradetsky opened this issue May 26, 2017 · 5 comments
Closed

lisp highlighting very bad #1533

dradetsky opened this issue May 26, 2017 · 5 comments

Comments

@dradetsky
Copy link

I was attempting to highlight the following block (from the sbcl compiler)

(defun find-or-create-constraint (kind x y not-p)
  (declare (type lambda-var x) (type constraint-y y) (type boolean not-p))
  (or (find-constraint kind x y not-p)
      (let ((new (make-constraint (length *constraint-universe*)
                                  kind x y not-p)))
        (vector-push-extend new *constraint-universe*
                            (1+ (length *constraint-universe*)))
        (register-constraint x new y)
        (when (lambda-var-p y)
          (register-constraint y new x))
        new)))

(which above, at least in my preview, is highlighted fine)

The block exists in a markdown document, and generates output that looks like <pre><code class="lisp">.... The code is highlighted by executing

  $('pre code').each(function(i, block) {
    hljs.highlightBlock(block);
  });

Note that I only import a handful of languages, and then call registerLanguage (as in this blog post)

Other languages are highlighted fine by highlight.js, but the lisp output is horrible. Basically, it highlights every symbol at the beginning of a list (with the exception of the 1+), and highlights them the same color.

The resulting html code block has class lisp hljs, which i take to mean that lisp was in fact used by hljs (I sometimes saw ruby hljs python on incorrectly highlighted stuff while i was getting this all working). If this is hljs working as intended, you do not have a lisp highlighter in any reasonable sense of the term.

FWIW, looking at the lisp example on highlightjs.org, 3 things jump out at me:

  1. At the top of the file is the shebang line #!/usr/bin/env csi which refers to the Chicken Scheme interpreter. Chicken Scheme, as the name suggests, is not lisp.

  2. The lisp code in the example is formatted totally incorrectly. It was clearly written by somebody who did not know lisp pretty-printing rules.

  3. The example is highlighted such that e.g. defvar and or (core language macros) are highlighted the same as prompt-read and parse-integer (definitely not part of common lisp; pretty sure they're not part of chicken scheme or any standard scheme impl). And at a glance it looks like the bound symbols of the let-expr are highlighted which is totally wrong.

Overall, I could be wrong, but it looks like you don't have a lisp highlighter. To the extent that other highlighters in that family are based on the lisp highlighter, you don't have highlighters for those languages either.

@dradetsky
Copy link
Author

FYI, I'm using version 9.11 via https://www.npmjs.com/package/highlight.js

@isagalaev
Copy link
Member

Let me start with this:

If this is hljs working as intended, you do not have a lisp highlighter in any reasonable sense of the term.

Please be reminded that an open source project is not a product, and nobody is interested in your "customer feedback" thrown in the air. This code is shared here to be used however you want without any obligations. You can improve on it and even share your improvements back if you feel like it.

Now, onto the specifics…

Note that I only import a handful of languages, and then call registerLanguage (as in this blog post)

This seems to be working, although it may not be the easiest way to do it. There's a section in the README on choosing the right size: https://github.com/isagalaev/highlight.js#getting-the-library (But if the above blog post works for you, it's fine)

The resulting html code block has class lisp hljs, which i take to mean that lisp was in fact used by hljs

That is correct.

Basically, it highlights every symbol at the beginning of a list (with the exception of the 1+), and highlights them the same color.

It's a bit smarter than that, depending on the concrete lisp variant, actually… As for the same color, there are different views on whether a lisp highlighter should make a distinction between built-in names and user-defined ones as the first item of an s-expression. Our definitions for Scheme and Clojure do make this distinction, generic Lisp doesn't. Also some coloring styles deliberately choose to highlight them in one color (Default), and some use different ones (Tomorrow).

At the top of the file is the shebang line #!/usr/bin/env csi which refers to the Chicken Scheme interpreter. Chicken Scheme, as the name suggests, is not lisp.

The lisp code in the example is formatted totally incorrectly. It was clearly written by somebody who did not know lisp pretty-printing rules.

Fixes are welcome.

The example is highlighted such that e.g. defvar and or (core language macros)

There are no defined built-in names for the generic highlighter for obvious reasons. You can write an SBCL (or CL) specific one based on that and those for Clojure and Scheme using them as examples.

@dradetsky
Copy link
Author

I'm aware of the nature of open-source projects. I'm sorry if that sounded a little harsh. I figured you'd want to know it didn't work correctly.

To be fair, I did originally test only with monokai, but the same problem is present in default, tomorrow[-night], and solarized-dark, although in some of those the numeral 1 in 1+ is clearly highlighted differently than the other highlight colors (although it probably shouldn't be, since it's a function, not a number).

I don't know what you mean by "differing views on whether..." I mean, I'm sure you can find somebody who holds that view. I just know it isn't shared by whoever wrote the syntax highlighters for emacs, vim, and whatever highlighter github uses. I've certainly never heard the view you mentioned, let alone seen it instantiated in the form of a syntax highlighter. I've seen a fair amount of highlighted lisp code. I'm only a sample of 1, but if the typical person who has seen lots of highlighted lisp code sees an instance of highlighting and believes it's an error, maybe it just is one by definition, even if some esoteric philosophy of lisp highlighting says it's not.

I mean, if I thought a syntax highlighter shouldn't distinguish between builtin functions and user-defined functions (or builtin macros and user-defined macros, or functions and macros), I'd probably just highlight none of them. I definitely wouldn't highlight the first symbol of every list (which just looks weird; changing parens color is the right way to make begin/end of lists more obvious). And if I did, I would either highlight the first symbol of every list which was a function application or the first of every list which was an unevaluated list (like the parameters to find-or-create-constraint), but not both. Are you starting to see why this looks indistinguishable from error?

I'd be happy to fix the example (although I don't know where the website repo is), but there seems to be little point so long as the actual highlighter remains broken.

As regards what I did, it came about as a result of using hljs inside a middleman-webpack stack (and not being much of a frontend or js developer), where the natural way to get your hands on stuff like hljs is to install the node package and import from, and allow this process to cut the specific minimal version you need. I used jquery because I found that turbolinks was breaking my highlighting and found to be a workaround which didn't require thinking anything through. I now realize (Like I said, I'm not much of a frontend/js developer) that a better solution would be to modify the core api to expose an initHighlightingOn method (of which initHighlightingOnLoad would be be a curried instance) to which one could pass anything, like 'turbolinks:load'. Unlike fixing the syntax highlighting, that's actually something I'm competent to do.

@dradetsky
Copy link
Author

dradetsky commented May 27, 2017

I originally wrote something more critical, but it turns out that after some further experimentation with themes, the scheme highlighter is in fact somewhat smarter and potentially usable (and can be substituted for lisp for the purposes of the point I'm trying to make). It does have an unfortunate tendency to highlight the names of builtins where they are used as variables, but I guess nobody's perfect (the vim highlighter does this too, although not the emacs highlighter). In any case, I would still suggest removing lisp from the list of supported languages, as I don't think it's fair to say it is supported, but it's your project.

@isagalaev
Copy link
Member

I'm not interested in this discussion.

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