molsonkiko / JsonToolsNppPlugin

A Notepad++ plugin providing tools for JSON like linting, querying, a tree view, and CSV conversion.
Apache License 2.0
70 stars 9 forks source link

Is there a way to turn off error correction? #51

Closed GTGeek88 closed 7 months ago

GTGeek88 commented 7 months ago

In trying to troubleshoot some JSON I'm sending to a website, I find that the pretty-print feature is handy, but not when it corrects errors. Granted, you have the (not always real visible) error reporting section, but I'd really rather not have it insert a comma where one did not exist. You can continue the pretty-print as if it was there, but just don't show it. Hopefully, I'm just missing a setting, because I really don't like it fixing what I did not have right to begin with. It doesn't help when I'm trying to find errors. Granted, maybe I should just look at the errors (a feature which I had overlooked until recently), but I think doing both is preferable (since your code could potentially miss something, too).

molsonkiko commented 7 months ago

I'm not particularly inclined to implement a pretty-printing method that deliberately leaves errors in the JSON. As you yourself noted, the error form exists if you want to manually correct errors. If you're pretty-printing to make JSON more readable, you could:

On Mon, Nov 27, 2023, 9:01 AM GTGeek88 @.***> wrote:

In trying to troubleshoot some JSON I'm sending to a website, I find that the pretty-print feature is handy, but not when it corrects errors. Granted, you have the (not always real visible) error reporting section, but I'd really rather not have it insert a comma where one did not exist. You can continue the pretty-print as if it was there, but just don't show it. Hopefully, I'm just missing a setting, because I really don't like it fixing what I did not have right to begin with. It doesn't help when I'm trying to find errors. Granted, maybe I should just look at the errors (a feature which I had overlooked until recently), but I think doing both is preferable (since your code could potentially miss something, too).

— Reply to this email directly, view it on GitHub https://github.com/molsonkiko/JsonToolsNppPlugin/issues/51, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALAQAI5IRD3KHDZBEXZWTKLYGTBPDAVCNFSM6AAAAAA74LMKPKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGAYTENZRGUYTSMI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

molsonkiko commented 7 months ago

Since it's not even really clear to me what you're asking for, and I have no intention of implementing what I think you're asking for, I'm closing this issue.

GTGeek88 commented 7 months ago

I didn't think of your plug-in as an error corrector. I did think of it as formatting what I gave it (as best it could), warts and all. You do make assumptions and correct problems and so it could format it, leaving the errors in while flagging them. The treeview seems to offer no help in this case. It displays the same way regardless of whether there's an error or not (at least the error I had). In my case, the error caused the treeview to not highlight the proper nodes in the JSON, even though it seemed to otherwise understand the structure. As an aside, this does alert me to an error in a roundabout way (though you might consider that a bug and fix it). The error list you generate is, of course, helpful, but it's not real obvious and I did not notice it for a good while. I'm not sure why you were unwilling to consider anything further or even wait for a reply from me before you closed this.

molsonkiko commented 7 months ago

In my case, the error caused the treeview to not highlight the proper nodes in the JSON, even though it seemed to otherwise understand the structure.

I didn't realize that you had an error in your JSON that was causing JsonTools to fail to parse your file.

If you are asking me to improve the parser, I would be happy to help, but you will have to give me an example so I can understand what the issue is.

I didn't think of your plug-in as an error corrector. I did think of it as formatting what I gave it (as best it could), warts and all.

You have a fundamental misconception of how the pretty-printing and compression in my plugin works. Allow me to clarify some things.

When my plugin pretty-prints or compresses your JSON, there are two separate stages:

  1. my plugin parses the JSON and creates a hierarchy of C# objects that represent the abstract structure of the file. During this stage, the parser notes any errors and comments it sees, but the objects it uses to represent the file's structure don't know anything about the errors and comments recognized by the parser. This basic setup exists for performance reasons, and I have no intention of changing it.
  2. my plugin uses the objects that represent the file's structure to output formatted text according to a compression or pretty-printing algorithm. When the plugin is pretty-printing or compressing the JSON, all that it knows is the abstract structure of the file. It has completely forgotten how the file was formatted before pretty-printing or compression.

It's not impossible to format a JSON in a way that remembers how the JSON was previously formatted, but it's a very hard problem, one that would require much more time than I want to invest in something that I regard as unimportant.

molsonkiko commented 7 months ago

This doesn't address all of the things you were dissatisfied about, but I've just made a change (available in the next release) that will make it so that a prompt asking if you want to see the error form is ON by default. There is a setting that controls this, but it was previously off by default because I thought people might find the prompt irritating.

molsonkiko commented 7 months ago

As a continued show of goodwill and interest in solving your problem, I have greatly improved the parser's handling of certain errors.

If your JSON had an error like this:

[
    {"a": 1, "b": "foo", // woops! missing a close '}' here
    {"a": 2, "b": "baz"}
]

previously JsonTools would fail completely and do something inappropriate.

My next release will correctly parse that JSON as [{"a":1,"b":"foo"},{"a":2,"b":"baz"}] as expected.

GTGeek88 commented 7 months ago

Thank you. My apologies for not responding, yet. Busy day with an upcoming deadline.RussellSent from my iPhoneOn Nov 28, 2023, at 4:40 PM, molsonkiko @.***> wrote: As a continued show of goodwill and interest in solving your problem, I have greatly improved the parser's handling of certain errors. If your JSON had an error like this: [ {"a": 1, "b": "foo", // woops! missing a close '}' here {"a": 2, "b": "baz"} ]

previously JsonTools would fail completely and do something inappropriate. My next release will correctly parse that JSON as [{"a":1,"b":"foo"},{"a":2,"b":"baz"}] as expected.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

GTGeek88 commented 7 months ago

Thanks for the changes. Still juggling a lot of stuff. I'm a little confused on the missing "}" example above. Will it correct it or put in something like the "whoops" message. Didn't know if you were saying that message would be there or if you just put it in your post here for clarity.

molsonkiko commented 7 months ago

No message would appear there; that comment was just for illustrative purposes (although Notepad++ actually does allow showing inline messages like that, though I don't know exactly how). If you pretty-printed the document, the missing } would be inserted

This type of error (missing } or ] in the middle of the document) could get one of any number of different error messages, depending on how exactly you make the error. This is because if a document contained a mistake, it's often hard to know exactly what the user intended, and thus it is also hard to say with certainty what type of mistake they made.

In lieu of further explanation, I'll just show you what errors are shown on the error form using my current (un-released) version of JsonTools on the example document.

image

The error messages are a bit confusing, but VSCode did a worse job of parsing the same JSON with even less useful error messages, so I figure that's a point in my favor.

If you want to know how I implemented the change and thus get a sense of what the plugin actually does when it encounters an error like this, you can see the changes to JsonParser.ParseArray, JsonParser.ParseObject, and JsonParser.ParseUnquotedKey in this commit.

GTGeek88 commented 7 months ago

Ok, thanks for the dialog on this. If you're doing a better job of parsing the JSON than VSCode, I guess that is good.