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

Support parsing for multiline string #184

Merged
merged 5 commits into from
Feb 17, 2023

Conversation

BOOMeranGG
Copy link
Collaborator

Fixes #46

@BOOMeranGG BOOMeranGG changed the title Multiline string Add parsing for multiline string Jan 28, 2023
@BOOMeranGG BOOMeranGG changed the title Add parsing for multiline string Support parsing for multiline string Jan 28, 2023
@orchestr7
Copy link
Owner

@NightEule5 you will get conflicts with this one PR, I guess… 🥲

@NightEule5
Copy link
Collaborator

@NightEule5 you will get conflicts with this one PR, I guess… smiling_face_with_tear

Ok with me

@NightEule5
Copy link
Collaborator

Nothing else to add, looks good to me. We'll see what akuleshov7 thinks, parsing is more his thing

} else {
TomlLiteralString(this, lineNo, config)
}
'\'' -> TomlLiteralString(this, lineNo, config)
// ===== basic strings
'\"' -> TomlBasicString(this, lineNo)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we received a multiline string and are parsing it with TomlBasicString?
It's ok, if it's not a mistake, but while reading the code - I would expect here different types of strings that start from ": Basic, Multiline, etc.

As this code was intended to be an entry point for determination of different types. So I got a little bit confused.

@@ -43,19 +46,25 @@ public class TomlBasicString internal constructor(
public companion object {
private fun String.verifyAndTrimQuotes(lineNo: Int): Any =
when {
// ====== multiline string (""") =======
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I see: you decided not to make differences on the higher level.
That's actually okay, may I should have done the same with LiteralStrings

Copy link
Owner

@orchestr7 orchestr7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked more closely, actually awesome work, lgtm!
Thank you.

PS great work with tests also

}
val value = line.substring(firstEqualsSign + 1).trim()

return value.startsWith("[") &&
value.endsWith("]").not()
if (value.startsWith("[") && !value.endsWith("]")) {
Copy link
Owner

@orchestr7 orchestr7 Feb 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only stylistic comment I have here - is usage of when statement

return when { 
    value.startsWith("\"\"\"") && value.getCountOfOccurrencesOfSubstring("\"\"\"") == 1 -> 
    value.startsWith("'''") && value.getCountOfOccurrencesOfSubstring("\'\'\'") == 1 ->
}

It usually looks much better with a huge number of if statements.

But basically not important

@orchestr7 orchestr7 merged commit 5252bf2 into orchestr7:main Feb 17, 2023
@BOOMeranGG BOOMeranGG deleted the multiline-string branch February 17, 2023 18:41
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

Successfully merging this pull request may close these issues.

Multiline literal strings
3 participants