DavidFeldhoff / al-codeactions

MIT License
17 stars 8 forks source link

AppSourceCop.json can include comments. #103

Closed rdebath closed 3 years ago

rdebath commented 3 years ago

In fact all JSON config files are supposed to allow comments. It would be best if you didn't choke on them.

  ERR Unexpected token / in JSON at position 37: SyntaxError: Unexpected token / in JSON at position 37
    at JSON.parse (<anonymous>)
    at Function.<anonymous> (c:\Users\Me\.vscode\extensions\davidfeldhoff.al-codeactions-0.2.25\out\extension\Utils\workspaceUtils.js:22:43)
    at Generator.next (<anonymous>)
    at fulfilled (c:\Users\Me\.vscode\extensions\davidfeldhoff.al-codeactions-0.2.25\out\extension\Utils\workspaceUtils.js:5:58)
    at processTicksAndRejections (internal/process/task_queues.js:94:5)

Remember a JSON config file is not untrusted content and so does not need a pathetically paranoid parser.

DavidFeldhoff commented 3 years ago

Thanks for the hint, I wasn't aware of that as I always got an error message then (even if the solution still compiled), so I double-checked it.

My findings: JSON itself does not support comments. But VS Code does. It has an own "jsonc (JSON with comments)" formatter as described here as well by Microsoft: https://code.visualstudio.com/Docs/languages/json#_json-with-comments

With that it works perfectly fine. I parse the AppSourceCop.json now as well with a jsonc-parser. It'll be shipped with v0.2.26 jsonc

rdebath commented 3 years ago

Thanks, but just BTW ...

I think you need to dig just a little bit deeper about JSON, it's a bit ugly.

According to Douglas Crockford, JSON is NOT supposed to be used for configuration files, it is a "data interchange format" for communication between programs that happens to be mostly readable (& writable) for people. This description is right in the first paragraph of the standard and was further clarified later.

Douglas Crockford has explicitly stated that comments are left out by design (so they can't be used to bastardise extensions into it) but if you insist on using JSON for a configuration file you should always use a jsmin program to strip comments out before giving it to a strict JSON parser.

https://web.archive.org/web/20120507093915/https://plus.google.com/118095276221607585885/posts/RK8qyGVaGSr

Personally, I think the old Microsoft "ini" file format, like git uses is best for config files, it doesn't have that arbitrary distinction between "tables" and "objects" or that weird requirement to explicitly put brackets round the whole file. Or quotes round the label names.

DavidFeldhoff commented 3 years ago

Thanks for the explanation, didn't know that so far :)

DavidFeldhoff commented 3 years ago

Should be working now in v0.2.26. Feel free to test and give feedback afterwards :) If I don't here anything from you, I'll close the issue after a week or so ;) All the best & Cheers David