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

Known issues #3

Open
Gabriella439 opened this issue Oct 1, 2016 · 7 comments
Open

Known issues #3

Gabriella439 opened this issue Oct 1, 2016 · 7 comments

Comments

@Gabriella439
Copy link
Owner

Most of the issues in this implementation are due to nix's support for string templating

My original first stab at this used Haskell's alex library to try to match the Nix lexing specification here:

https://github.com/NixOS/nix/blob/master/src/libexpr/lexer.l

However, the problem is that alex lexers only support a single numeric state, whereas the nix lexer requires a stack of states in order to handle arbitrarily deep nesting of templated strings, like this:

"${"${"${"ouch"}"}"}"

This can still be done in alex (by bypassing alex's state support and keeping track of the stack of states out-of-band in a custom monad), but this requires more work. Since I wrote the first draft in the context of a hackathon I didn't have enough time to implement that stack. Instead, I just implemented everything entirely in the parser, which led to most of the issues that this implementation has. Specifically:

  • I didn't correctly implement all the rules for parsing '' string literals
  • Parsing string literals is really slow and fails hard on long strings

If we can fix this to use a proper alex lexer it would significantly speed things up and fix most of the correctness issues, too.

I have the beginnings of the Alex lexer in version control, so you can see it here:

https://github.com/Gabriel439/nixfmt/blob/1c13114f3e7faf4ada5310d23deb48678087c9df/src/Lexer.x

... and you can also see where I gave up right around here in lexing ${:

https://github.com/Gabriel439/nixfmt/blob/1c13114f3e7faf4ada5310d23deb48678087c9df/src/Lexer.x#L47

... which is where I needed to support a stack of states

cc: @domenkozar, who asked about this

@copumpkin
Copy link

From a quick glance over the Main.hs, it also appears that this project doesn't handle comments yet, right? There are some interesting/difficult issues that code formatters need to deal with around comments, since most lexers discard comments very early on, and it can be tricky to "pin" a comment to surrounding content.

It can also be interesting to preserve a limited amount of whitespace in the generated AST, since most developers have some sort of distinction they make between no empty lines, one empty line, and (occasionally) two empty lines between things. More than two could get condensed into two.

Anyway, this is definitely a promising project but I wanted to make sure the bill o' work for any prospective contributors included things that I think can be tricky to get right 😄

@domenkozar
Copy link
Contributor

@Gabriel439 do you have an estimate to supported nested syntax such as comments and antiquotation? I'd be happy to run a funding campaign here to make this happen. I sadly don't have the knowledge nor time to get it going, but I'm sure we can get some sponsorship.

@PierreR
Copy link

PierreR commented Nov 22, 2016

FWIW I would be happy to help in my free time when the foundations are in place.

@Gabriella439
Copy link
Owner Author

@domenkozar Money is not the issue, just time :)

Right now my highest priority is completing my Dhall compiler and once that's done I'll begin revisiting other projects, including this one

@domenkozar
Copy link
Contributor

@Gabriel439 appreciated. I know money is not the issue, but it can help with prioritization. Let me know and really looking forward to have nix formatter/prettyprinter!

@domenkozar
Copy link
Contributor

@Gabriel439 is this project still on your radar?

@Gabriella439
Copy link
Owner Author

No. This was just a Hackathon project I uploaded. If I were to revisit this I would do it a different way anyway

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

4 participants