search

nix-community / rnix-parser

A Nix parser written in Rust [maintainer=@oberblastmeister]
MIT License
365 stars 44 forks source link

Correctly remove multiline trailing whitespace #103

Closed darichey closed 2 years ago

darichey commented 2 years ago

Summary & Motivation

Previously, trailing whitespace on the last line of a multiline string was always removed, but this is not necessarily correct. For example, the string ''hello '' should keep its trailing whitespace.

This PR attempts to mimic the logic used in the reference implementation of the parser to instead only remove the final line if it consists solely of whitespace.

Ma27 commented 2 years ago

If we do this, we should do it properly from the start, i.e. remove all whitespace prefixes. In fact I have started on a patch for that, but it needs some more polishing, expect a PR ~next week :)

fogti commented 2 years ago

don't we already do that? The only difference necessary is that the first and last line are handled differently (in them, if they contain only spaces ' ', they're omitted)

darichey commented 2 years ago

Right, I believe that is already handled. That said, I also think that this entire logic could use a rework. I started by trying to just dumbly port the reference implementation in this commit: https://github.com/darichey/rnix-parser/commit/d56b15ff4ae9e1c03a28c1f0894f6881761925dd

I confirmed that it at least parses all multiline strings in nixpkgs (see #107), but honestly the code is kind of ugly, and I haven't done any kind of benchmarks, so I didn't bother making a PR yet. Please feel free to use it as inspiration @Ma27 :)