chris48s / v8r

✔️ A command-line JSON, YAML and TOML validator that's on your wavelength
https://chris48s.github.io/v8r/
MIT License
28 stars 5 forks source link

allow parsing multi-doc yaml files #498

Closed chris48s closed 2 weeks ago

chris48s commented 2 weeks ago

Closes https://github.com/chris48s/v8r/issues/378

Changelog:

chris48s commented 2 weeks ago

Here's an issue I didn't think of.

I need to output something sensible in getSingleResultLogMessage()

One way I could deal with this would be:

   getSingleResultLogMessage(result, fileLocation, format) {
     if (result.valid === false && format === "text") {
-      return formatErrors(fileLocation, result.errors);
+      return formatErrors(`${fileLocation}[${result.documentIndex}]`, result.errors);
     }
   }
 }

That is simple and easy, but it means I'll switch from saying

./package.json#/version must be string

to

./package.json[0]#/version must be string

in all cases. It would be nice to avoid that given multiple documents in one file is quite an uncommon case.

I could do this:

     let results = [];
     for (const filename of filenames) {
       const fileResults = await validateFile(filename, config, plugins, cache);
+      const multiDoc = fileResults.length > 1;
       results = results.concat(fileResults);

       for (const result of results) {
         for (const plugin of plugins) {
+          const fileLocation = multiDoc
+            ? `${filename}[${result.documentIndex}]`
+            : filename;
           const message = plugin.getSingleResultLogMessage(
             result,
-            filename,
+            fileLocation,
             config.format,
           );
           if (message != null) {

but it feels like a bit of a hack and means the content of fileLocation is inconsistent in different places.

I could make some kind of breaking change to getSingleResultLogMessage() that allows me to pass in a variable telling me if the file is a multi-doc.

Finally, I could indicate this in some way in the ValidationResult object. One way would be to add a specific property. The other thing I could do is say that documentIndex is null if the file only contains a single document and only populate it with a number when it contains >1. I think on balance that is probably the nicest approach.

chris48s commented 2 weeks ago

reckon this is probably good. Needs another read over with fresh eyes though

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 98.40000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.60%. Comparing base (3a9c9ed) to head (6342e8c). Report is 8 commits behind head on main.

Files Patch % Lines
src/cli.js 98.87% 0 Missing and 1 partial :warning:
src/plugins/parser-yaml.js 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #498 +/- ## ========================================== + Coverage 96.40% 96.60% +0.19% ========================================== Files 20 20 Lines 1281 1354 +73 Branches 268 283 +15 ========================================== + Hits 1235 1308 +73 Misses 45 45 Partials 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.