feat: implement multiline support (continued)

[joshuakb2]

I saw #51 and thought it was a good idea, so I thought I'd try to get it over the finish line.

I rebased aurelien-brabant's commits onto main as they had fallen behind. I fixed a typo in his code and changed the RHS index to use std::string::at like you suggested.

what if line.find_last_not_of(MULTILINE_SPACE_CHARSET) returns std::string::npos

This appears to be a non-issue. In that case the code is evaluated as line.substr(0, 0).

I also added another multiline test for configs that contain a whole empty line in the middle, and I fixed a bug where the RHS was assumed to be non-empty.

Let me know if you still have concerns about this feature and I'll see if I can address them.

[vaxerski]

considering multi-line stuff can only happen when parsing a config file, I am in favor of moving them out of the ::parseLine func altogether. This adds needless complexity.

I would instead move it to parseFile and parseRawStream

[joshuakb2]

In that case, I'll take another look at this change and see about implementing it the way you're describing. Thanks for the quick feedback!

[joshuakb2]

I implemented a simpler version of multi-line support in parseFile and parseRawStream.

Functional changes from the previous implementation:

• The \ at the end of the line may not be followed by a comment or whitespace.
• Lines ending with > are not treated specially at all.

I should also point out that this feature is a breaking change because people may have config lines ending with backslashes where those backslashes are currently treated as normal text. But this was true of the previous implementation as well.

I welcome any feedback, and thanks again for considering this pull request!

[joshuakb2]

I consolidated the logic for getting the full next line (taking backslashes into account) into a helper function that returns std::expected like you suggested. What do you think?

[vaxerski]

rest lgtm

[joshuakb2]

Cool! I've addressed those changes you asked for. Thanks!

[vaxerski]

rest lgtm

[vaxerski]

this is not used outside of config.cpp, this can be removed altogether

[vaxerski]

You don't need to define stuff in a header if it's only used in one source file, it only increases the compilation time

[joshuakb2]

Alright, I've fixed that now. Thanks for the feedback, I hadn't considered that. I don't write much C++ these days but it's good to get a little practice in.

[vaxerski]

thanks!