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

Issue with Elixir highlighting #707

Closed
whatyouhide opened this issue Jan 11, 2015 · 17 comments
Closed

Issue with Elixir highlighting #707

whatyouhide opened this issue Jan 11, 2015 · 17 comments
Labels
Milestone

Comments

@whatyouhide
Copy link

This HTML snippet:

<html>
  <head>
    <link rel="stylesheet" href="highlight/styles/default.css">
    <script src="highlight/highlight.pack.js"></script>
    <script>hljs.initHighlightingOnLoad();</script>
  </head>

  <body>
      <pre><code class="elixir">defmodule Foo do
  def foo, do: :foo
  def bar, do: :bar
end</code></pre>
  </body>
</html>

produces this buggy result:

screen shot 2015-01-11 at 22 23 25

because of the def f, do: # code syntax (as an alternative to the usual do/end block, which works just fine). I think @knewter took care of the Elixir highlighting, so if he has a quick fix that would be great! Otherwise, I'll try to start working on it as soon as I can. I never contributed to highlight.js though, so it could take me a while to get familiar with it 😞.

@Sannis Sannis added the bug label Jan 12, 2015
@Sannis
Copy link
Collaborator

Sannis commented Jan 12, 2015

Am I right that with this syntax function body ends with new line? (\n)?

@whatyouhide
Copy link
Author

@Sannis I don't think so. The do: is followed by an expression. Some examples of valid Elixir code:

# This reproduces the bug
def foo, do:
  3 + 5

or

# This is highlighted perfectly fine
def foo, do: 3 +
             4 +
             5

@tonini
Copy link

tonini commented Jan 28, 2015

@Sannis @whatyouhide What is the status about that? we really would like to fix this because we love to use it in ex_doc :-)

Before I'm starting looking into it by myself, maybe you guys can have a quick look before me. :)

@knewter I know you have a lot to do, but if you got a few minutes, please have look. ❤️ :-)

@whatyouhide
Copy link
Author

Any news on this? As @tonini said, we would really really love to fix this since it's screwing up a lot of examples in Elixir's documentation (as well as in the documentation of every library using ex_doc, i.e., every library 😄).

@knewter
Copy link

knewter commented Jan 30, 2015

Urk, what's the situation with the tests these days? src/test.html isn't
a thing anymore, and test/index.html doesn't really trigger anything
(don't know that it should). Is there documentation on how to add new
language support re: testing/verifying? Because I was going to fix this
today and realize it's changed too much for me to get it done in 15 minutes
or so :(

On Fri, Jan 30, 2015 at 9:06 AM, Andrea Leopardi notifications@github.com
wrote:

Any news on this? As @tonini https://github.com/tonini said, we would
really really love to fix this since it's screwing up a lot of examples
in Elixir's documentation (as well as in the documentation of every library
using ex_doc, i.e., every library [image: 😄]).


Reply to this email directly or view it on GitHub
#707 (comment)
.

Josh Adams
CTO | isotope|eleven http://www.isotope11.com
cell 215-3957
work 476-8671 x201

@isagalaev
Copy link
Member

@knewter the docs on testing are here: http://highlightjs.readthedocs.org/en/latest/building-testing.html (in short, we now have tools/developer.html to play with markup).

I'm not sure when any one of us will be able to look into it (for me it's probably next Tuesday), so feel free to pick it up.

@knewter
Copy link

knewter commented Feb 4, 2015

I'm looking at it now, we'll see if anything comes of it /cc @tonini @whatyouhide

@knewter
Copy link

knewter commented Feb 4, 2015

so @tonini @whatyouhide @josevalim I have a few questions about this one. In this case, isn't do an atom in an optionlist? That's the core problem here - it's matching "thing: other" such that thing is an atom in the list. That's matching do. I can change the regex such that do isn't matched, in which case it would become a keyword...but then you would always get that behaviour if you used do: in an optionlist as well, which is wrong (but better?) Basically, I need to do this work when @josevalim or someone is around to help explain to me my misunderstandings like this in the parser. Sadly the highlight.js syntax checking is in no way presently easy to 'port' from the actual tokenizer in elixir, primarily due to my lack of experience here (only parsers I've really written that were complicated were PEGs)

@whatyouhide
Copy link
Author

@knewter I think the problem is not that do: isn't highlighted as a keyword (it's not, it's just an atom and that's fine). The problem is that all newlines after a do: get messed up.

Note that this is valid Elixir syntax:

%{a:
  3}

or

[do:
  something]

@knewter
Copy link

knewter commented Feb 4, 2015 via email

@isagalaev
Copy link
Member

Am I getting this right that in the case of

def foo, do: :foo

The comma , actually signals the end of the function header and that do: is something else entirely than in the case of

def foo do
  # ...
end

?

@eksperimental
Copy link
Contributor

@isagalaev what comes after , do: should be treated the same way as what's in between do and end, with the exception that that , do: cannot accept new lines, and ; and ( ) are used instead.
I don't know if that answers your question.

def foo, do: (IO.puts "hello"; IO.puts " world")

produces internally the exact same code as:

def foo do
  IO.puts "hello"
  IO.puts " world"
end

@isagalaev
Copy link
Member

Ooookay, I think I've fixed it, finally:

elixir-functions

I even had to introduce a new functionality in the core for that :-) Which I hope is going to be needed for another bug elsewhere.

@josevalim
Copy link

Awesome, thank you!

@eksperimental
Copy link
Contributor

much appreciated @isagalaev !

@whatyouhide
Copy link
Author

Awesome @isagalaev, thanks so much! ❤️

@knewter
Copy link

knewter commented Mar 19, 2015 via email

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

No branches or pull requests

7 participants