add possibility to parse toml files like pyproject.toml

[BenniWi]

add functionality to read in *.toml files like the pyproject.toml as requested in #113.

further: add small fix to js and json config file

[DavidAnson]

JSON and YAML configuration files store everything in the root-most object. This PR proposes that TOML files would be required to store everything in an object twice nested (toml.markdownlint). I understand this is being done for the sake of an external tool, but it is inconsistent for users of this CLI. Note that CLI2 supports configuration embedded in Node's package.json, but does so as an additional format and not as a requirement for all JSON files. I suspect that won't be enough for you and the folks who opened the corresponding issue, so perhaps it would make sense to add two new entries to the parsers array: one that behaves like you've done here and one that looks for the object at the root? I don't love my proposal, and am open to other ideas.

[BenniWi]

I see your point but agree, that checking for both might not be the best solution.
An alternative approach would be to have an additional command line argument to specify in which level to search for the configuration data !?
What do you think about that?

[DavidAnson]

I wasn't sure at first, but this idea grew on me when I realized that what you propose would work for any of the configuration file formats! Specifically, your proposal could also be used to enable storing settings inside Node's package.json (similar to what CLI2 allows) - with no additional work.

So my suggestion is that configuration is always assumed to live at the root of all configuration objects (as is already the case today) and you add the new parameter you propose which optionally provides a path into that object for scenarios like yours.

I propose using the JSON path syntax described here: https://www.ietf.org/archive/id/draft-ietf-jsonpath-base-01.html. However, I do not think we need to support the full syntax at this point. Instead, I propose the only syntax that should be allowed is "$.first.second" for an arbitrary nesting. "$" alone should be the default and maps to today's behavior. A simple string split on "." should be all the implementation that's needed for now which keeps this simple but leaves the door open for more sophistication in the future.

This makes the proposed change much more work – especially around new test cases. I understand if you're not ready to get into that now, but favor this as the path forward barring other/better suggestions.

[BenniWi]

Hi @DavidAnson,
thanks a lot for the fast and detailed answer.
I fully agree with your proposal and would try to implement.
I'm quite new to js and I think this might take some days for me to realise.
I will come back to this discussion as soon as everything is done.

[DavidAnson]

Cool, good luck! I tend to be very picky, so let me warn you and apologize in advance. :)

If you'd like to post updates as you make progress, I'd be happy to have a look at those and make sure we stay on the same page. It's a lot easier than waiting till the very end and finding out we had completely different ideas.

Thinking briefly about the work here, I don't think there is a lot of new code. You can use the existing command-line parser to add a new single-value parameter. You already found how to add the new TOML parser. The big thing will be test cases that cover the new behavior for the various formats.

One approach you might take - and I might even suggest - is to add the TOML parser in a single self-contained commit. That's basically the work you have already done except that you'd nest everything at the root and remove the nesting. Otherwise, I think what you already have here is close to ready and we could complete this PR for the feature "add TOML parser". Then you can do the work to add the new command-line parameter in a second PR.

It can help to split things so there are fewer moving pieces. Just a thought!

[BenniWi]

I switched everything to root now and raised the new issue #458 to add the additional command line argument.

Unfortunately I struggle a bit with the toml parser in general.
The configuration is correctly parsed, but in the further process, the linting is not done at all. I couldn't figure out the problem so far.
I observed, that for json/yaml parsers, the config variable contains a prototype object, not only the configuration itself. For toml this is not the case. Don't know if this is somehow related.

[DavidAnson]

Everything here looks like I expected. I've left a few comments - only one real point of discussion.

That said, can you please explain your comment, "Unfortunately I struggle a bit with the toml parser in general. ..."? I don't understand the problem - as I said, everything here looks like I expected. What isn't working for TOML that works for JSON/YAML?

[DavidAnson]

Thanks for the updates!

[BenniWi]

Hi @DavidAnson, thanks a lot for your time and for guiding me through this.
It is my first time working with JS and I really appreciate your help.