Skip to content

Conversation

@chelog
Copy link

@chelog chelog commented Nov 30, 2021

Fixes edge case with value ending with a backslash, even escaped:

"UserLocalConfigStore"
{
	"friends"
	{
		"9734658" "Grocel"
		"5318652" "/Hexalitos\\" <-- this one
		"19358028" "put in"
	}
}
SyntaxError: VDF.parse: invalid syntax on line 8:
"

As an additional bonus, adds strict comparison in this line where it's appropriate

Fixes edge case with value ending with a backslash, even escaped:
```
"UserLocalConfigStore"
{
	"friends"
	{
		"9734658" "Grocel"
		"5318652" "/Hexalitos\\" <-- this one
		"19358028" "put in"
	}
}
```
```
SyntaxError: VDF.parse: invalid syntax on line 8:
"
```

As an additional bonus, adds strict comparison in this line where it's appropriate
@p0358
Copy link
Owner

p0358 commented Dec 23, 2021

Unfortunately in my tests this still breaks. Analogous to your example:

edge_escaping_case {
    "problematic1" "/aaa\\"
    "problematic2" "/aaa\\"//commment
    "ok" "ok"
}

gets parsed as

edge_escaping_case: { problematic1: '/aaa\\"\n', problematic2: ' ', ok: 'ok' }

Then pasting your exact example (minus the <-- this one ofc) into my tests at the end seems to yield syntax error either way. Which makes this kinda impossible to merge even as a temporary mitigation, because it doesn't seem to improve things much.

I did dig deeper into KeyValues syntax and parsing and found out it can be switched into two different parsing modes related to escape sequences, so sadly parsing this properly and consistently with Source will require a bigger and more thorough update*. I'll need to sit on this again and do it finally, maybe tomorrow...

*which I tried to do some weeks ago, but put it away after a few hours when my head started aching xd

@chelog
Copy link
Author

chelog commented Dec 27, 2021

Well these regex'es are pain in the ass agree haha. I rewrote the lib in TS so maybe there are some other changes that fixed as mine app works fine. I'll have a look later and update the PR

@chelog
Copy link
Author

chelog commented Mar 27, 2022

@p0358 I just stumbled upon this old PR and gave a chance to retry the tests. I think you made a mistake and used JS string like that:

Which is actually this for VDF:

Obviously this fails to be parsed.


Correct test is this:

And this fails for your master branch and works well for my branch in PR. So I hope this gets merged

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.

2 participants