elm-ast
elm-ast copied to clipboard
Bogdanp
Comments make parsing fail
f =
let b = 1 --comment
in b
The parser does not understand this.
This is probably related to #1, because this, too has to do with newlines.
Yes. Comments right now are only statements (outside of functions). We're working on a way to go past that, but it's not trivial
It may be helpful to do multiple passes of the source. One of which might be filtering out comments.
Yeah. The parser doesn't really have a capability for us to do it. In elmchemy right now we're removing inline comments before we parse the code.
I think it would make sense to move the code from Elmchemy here. I don't see why someone would want the parser to fail instead of stripping comments.
@joonazan But we aren't really parsing them, we just ignore them. So that's probably not a desirable behaviour for a parser 😁
It would be awesome if this can be resolved. Elm-Ast is already providing a lot of value.
Another minimal example that causes a failure:
test : Int
test =
--
1
I did not see a lex combinator defined or used in this codebase. When I look at parsers for other languages I frequently see a lex combinator used to remove the whitespace and comments. Here is an example of parsing JSON using a combinator library in rust https://github.com/Marwes/combine/blob/master/benches/json.rs.
Something like:
lex : Parser s r -> Parser s r
lex p =
p <* (skipMany (whitespace <|> comments))
Then just wrap just about every low level parser with lex.
But we aren't really parsing them, we just ignore them. So that's probably not a desirable behaviour for a parser 😁
@wende to your point, since this is not parsing these statements but throwing them away, what if we had an alternative function that acknowledged this? Something like Ast.discardCommentsAndParse? It could do a first pass to throw out comments, and then do the parse to ensure comments don't cause parsing errors. This issue seems to be causing some errors in people's code so it would be nice to offer a workaround.
@dillonkearns I like the idea to make it configurable. Could fit the next major version :+1:
@wende that would be outstanding! It would really help me unblock some of my users in https://github.com/dillonkearns/elm-typescript-interop.
Hey @wende any update on this? I still have some users who are blocked on this, it would be great to be able to have some workaround to help strip comments before parsing.
FTR: @tunguski has a fork here: https://github.com/tunguski/elm-ast that seems work well with comments. At least the example page https://tunguski.github.io/elm-ast/example/ parses the code from https://github.com/Bogdanp/elm-ast/issues/41#issue-229655914
https://github.com/tunguski/elm-ast/issues/3 refers to https://github.com/tunguski/elm-ast/commit/60a771c2ae3e3fac5185b2bbfb2e14e3af72d2bf
Hello @wende, it looks like comments are parsing successfully now! Thank you so much for adding that in. Both -- ... and {- ... -} comments are parsing as expected, so this looks to be fully resolved now.
Would you mind closing the issue so it's clear for other users who were waiting for this? Thank you!
I apologize for the confusion, the examples I tried for parsing comments were not within function bodies (like in this example). So it turns out the behavior hasn't changed and comments like these still cause parsing to fail.
I did indeed find that https://github.com/tunguski/elm-ast resolves the issue, as @muelli suggested (thanks for that tip @muelli!).
I would still love to see this get resolved in this package! It would be nice if we could avoid having too much fragmentation in the Elm AST library ecosystem.