Elmx produces invalid multiline strings #16
Comments
|
This effectively breaks elmx :( – please merge @jakub-zawislak's PR! |
|
Dang, it's been a year and a half. Is there any reason not to merge jakub-zawislak's PR @pzavolinsky? I'd like to use elmx in my project. I guess I can just clone this repo and merge the PR myself rather than getting the dependency from npm. |
|
Hi @joshuakb2 sorry this flew under my radar. As I explained here I'm not actively using ELM and I doubt I can maintain this package on my own. When I initially reviewed the PR you mention (long ago), the changes, although pretty minimal, broke some tests (hinting at a possible bug, e.g. breaking everything but multi-line strings). Again, since I'm not an ELM user anymore, I'm not sure if this is the expected behavior nowadays, but my gut suggest it's not. In any case, the issue reported by @danneu is surely a bug, and if anyone wants to help with the code, either by building up from this PR or by starting from scratch, I'd be glad to review and merge. Sorry it took me so long to reply, I'm currently focusing in other projects and it's hard for me to keep up with issues in this one (as you obviously know already). I apologize for not being on top of this, and I kindly ask anyone interested in moving this project forward to help by either submitting PRs and/or reviewing them. Cheers! |
|
Hi @pzavolinsky, Sorry, I didn't think to look at the PR until after commenting, where I noticed that you mentioned the failed tests. Jakub's PR fixes the bug by simply using triple quotes for all string literals, regardless of whether or not they contain newlines. The tests simply failed because they were not updated to expect triple-quote string literals. I cloned his PR and fixed the tests, so I can submit a new PR if you think Jakub's solution is well-conceived. Otherwise, do you think we should continue to use single quotes when strings do not contain any newlines? Thanks for the quick response! |
Awesome!
Honestly I don't feel qualified to answer this. I guess it depends on what ELM specifies and I'm super rusty on ELM. If we could get some consensus out there on what should be done, I'll listen to what the ELM community thinks best. I know is asking too much, but if you could check some docs or other ELM projects to see what should be done that would be great. Cheers! |
|
I had trouble finding any guidance online, but in general I think it's better to use standard string notation when a string doesn't need multiline notation (for better readability), and luckily, implementing that behavior turned out to be very easy. PR incoming. |
produces
(error: unexpected
\n)of course, that could be fixed with
for some reason it seems like elm switched from single-pair to triple quote multiline strings in the last year, and i can't find a changelog. maybe i'm going crazy.
The text was updated successfully, but these errors were encountered: