Add other Clojure accepted whitespace to reader

[Tko1]

File affected: reader.rs

Reader right now is

1. Often using nom's ws macro to consume whitespace around a read, and to my knowledge this is deprecated.

2. Only looking for traditional whitespace. In Clojure, however, , is considered whitespace as well, and this case needs to be added, as well as any edge case I've never heard of if one exists

So, the preferable solution is to create a parsing function for clojure whitespace, ie something like

// This is named clojure_whitespace rather than whitespace to emphasize this is consuming
// what Clojure considers to be whitespace, including the , 
// Am open to a better name 
pub fn clojure_whitespace_parser(input:&[u8]) -> IResult<&[u8], String> {
   ...
}

And then, instead of combining this with another parser with nom's macros like

    named!(someParserThatConsumesWhitespaceFirst,
	   ws!(tag!("}")));

We would use some function combinator, like preceded

let someParserThatConsumesWhitespaceFirst = preceded(clojure_whitespace_parser, ... )

This would involve updating all spots that currently consume whitespace, which should be

1. try_read
2. try_read_map
3. try_read_string
4. try_read_vector
5. try_read_list

[scrabsha]

Hello,
I am interesting on working on this issue. As it is my first real contribution to open-source, I think the code which will be proposed in my first PR won't be perfect.

Also, I have quickly looked at clojure references, and it seems that, apart from traditional whitespaces, , is the only new whitespace (see here).

About ws:
As you stated, ws is being deprecated, and is not included in nom-6.0.0-alpha1.

Can this issue be assigned to me?

Edit: add information about ws.

[Tko1]

Sure that sounds fine, I appreciate that and don't worry, I'll refine the PR afterwards

[scrabsha]

Thanks!

[scrabsha]

I am currently implementing the function clojure_whitespace_parser, and I have some concerns about its return type. Do we care about the consumed whitespaces? To me, it would make more sense to return IResult<&[u8], ()> instead of `IResult<&[u8], String>.

Additionally, converting a sequence of bytes (&[u8]) to String may fail (String and str are guaranteed to be valid UTF-8, &[u8] is not). In this case, it can't fail, since we are only consuming ascii whitespaces (and ,), but the conversion involves some checks which might not be necessary.

[Tko1]

Indeed we don't care about its return type, I originally copy and pasted a random parser for my example and realized the same thing when I came back to this post and noticed it specifically returned a string. Good catch

Another area I will possibly start a ticket on is the parsers taking &[u8] to begin with; this was originally not a deliberate design decision, it just happened to be what worked the most cleanly for my 'first draft' of the reader. Trying to implement everything as parsers on &str, as I originally wanted, sometimes worked and sometimes didn't work well at all, and while the reasoning is likely something small, I figured that'd be a good area to come back to later. Granted, in the grand scheme of things it may not matter -- everything being fed to the reader, and thus then the parsers, starts off as a string before being converted into a byte slice, never are some raw potentially-invalid bytes being fed in instead from the top, and I would not forsee this practically becoming some mysterious bug -- but regardless, a &str is really the right reflection of our intent.

[scrabsha]

If you're okay with it, then I'll be very happy to implement it!

That was supposed to be my next comment, by the way.

[Tko1]

consume_clojure_whitespaces ended up breaking reading, as since its defined in terms of take_while1 (ie,

named!(parser, take_while!(is_clojure_whitespace));

), it will actually fail if there is no whitespace. Even though it technically won't consume any whitespace if none is there, we don't want it to fail when that happens, it still did its job (similar to a cashier who gets no customers) so I've changed it to take_while.

[scrabsha]

You're right. I apologize for that.

[Tko1]

You don't ever have to apologize for something like that lol I wrote it here to let you know and document the change, but I would never be upset over a contribution