[WIP] Config parsing #40
Conversation
…next up is bringing in the correct output types.
|
Hi @avitex, I finally pulled myself together to face my fears: Write the git config parser using Thus I am super interested to hear what you think, and now I will go and point out a few places where I think I might be doing something wrong. Thanks in advance ❤️ |
| @@ -30,6 +30,15 @@ impl From<Span> for Range<usize> { | |||
| } | |||
| } | |||
|
|
|||
| impl From<Range<usize>> for Span { | |||
There was a problem hiding this comment.
Since dangerous uses a Range<usize> I might just use this instead of an own version of ..=. Probably I had a reason to use an inclusive range in the first place, but it's not entirely clear to me right now what that was 🤦♂️.
| let mut tokens = Vec::new(); | ||
| skip_whitespace_or_comment(r, ConsumeTo::NextToken).map(|section| { | ||
| tokens.push(spanned::Comment( | ||
| dangerous::input(section) |
There was a problem hiding this comment.
In theory this conversion would have to be done for each token we parse here. Certainly this can be sugared up a bit more to feel less cluttered, but might also be a case for just using zc.
After all, the File type really is an zc type, owning data and keeping offsets into the data. The data will always be read into memory I think, so that would match too (as opposed to memory mapping them).
| } | ||
| let parsed = r | ||
| .take_consumed(|r| skip_whitespace_or_comment(r, to_where)) | ||
| .as_dangerous(); |
There was a problem hiding this comment.
Here I thought there could be an as_dangerous_opt(), as I specifically avoided doing as_dangerous_non_empty().ok() probably due to a very premature optimization. Generally I like the _opt() versions of methods a lot.
| } | ||
|
|
||
| #[must_use] | ||
| fn skip_whitespace_or_comment<'a, E>(r: &mut BytesReader<'a, E>, to_where: ConsumeTo) -> Option<&'a [u8]> { |
There was a problem hiding this comment.
Yes, this was a little lazy, and this should really be refactored to not need the inner function. One would probably end up with something cloning the input, but… I don't actually know how to do this properly.
|
Hey @Byron, awesome to see this! I'll check this out tonight and give you any feedback! |
…as some of the new features are very much what's needed here.
|
Note that this branch will be replaced with #44 when it exits that draft status. |
Generally there will be many small commits and a lot of 'sketching' in an effort to try find good ways of doing things, and iterate until it feels right, before going into all kinds of depths and details.
Thus there should be the first steps of basic parsing along with some pieces of editing and (re-)serialisation of config files to jiggle the API into something ergonomic and idiomatic.