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

Skyhigh173/JSON: fix json_is_valid() #1596

Merged
merged 2 commits into from
Aug 5, 2024
Merged

Conversation

Procybit
Copy link
Contributor

@Procybit Procybit commented Jul 8, 2024

The function returns false when valid checked JSON is not whitespace-trimmed. Made it check parsed type.

The function returns false when valid checked JSON is not whitespace-trimmed.
Made it check parsed type.
Copy link

@RockyTheProtogen RockyTheProtogen left a comment

Choose a reason for hiding this comment

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

Code no longer returns false when whitespace is introduced.
Fulfils PR but could break existing projects.

@CubesterYT
Copy link
Collaborator

I don't think it'll break projects considering projects would've already accounted for this bug and used non whitespace valid json

Copy link

@RockyTheProtogen RockyTheProtogen left a comment

Choose a reason for hiding this comment

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

After a bit of thinking, this shouldn't really affect anything unless projects, for some reason, use a rudimentary way of reading the JSON. Though that is doubted, as the get key block is in this extension.

@GarboMuffin
Copy link
Member

GarboMuffin commented Jul 11, 2024

this changes the output of

image

what do you think that should output?

true false 0 "hello" are all valid JSON too, should those not return true here as well?

regardless this will need to be a is_valid_v2 kind of situation since it does affect behavior in ways that we can't really predict

@LSPECTRONIZTAR
Copy link

uh this problem is already addressed in #1212

@Procybit
Copy link
Contributor Author

this changes the output of

image

what do you think that should output?

true false 0 "hello" are all valid JSON too, should those not return true here as well?

regardless this will need to be a is_valid_v2 kind of situation since it does affect behavior in ways that we can't really predict

Wouldn't additional check for null solve the problem? Previous version worked similar excluding null value, and relied on begin/end characters.

@GarboMuffin GarboMuffin linked an issue Jul 24, 2024 that may be closed by this pull request
@Skyhigh173
Copy link
Contributor

Just trim trailing and leading white spaces and test it with original script

@Procybit
Copy link
Contributor Author

Procybit commented Aug 5, 2024

So for now it just trims given JSON string before processing it

@GarboMuffin
Copy link
Member

okay, i think these blocks need to be replaced but this should be an unambiguous improvement for now

@GarboMuffin GarboMuffin merged commit 3ce22a6 into TurboWarp:master Aug 5, 2024
3 checks passed
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.

JSON - objects with multiple lines are not recognized properly
6 participants