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

feat: Add JSON path to stylish formatter #1382

Merged
merged 5 commits into from
Nov 20, 2020

Conversation

aburgel
Copy link
Contributor

@aburgel aburgel commented Oct 23, 2020

When running the linter against a generated OpenAPI document, line numbers aren't all that helpful in seeing where the source of an issue is coming from. In my case, we generate the doc in a CI build, so don't easily have access to the generated file. We do have access to the CI output, and having a JSON path means we can see what path is producing the error.

So this PR adds the JSON path to the stylish formatter makes it easier to see the cause.

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

Screenshots
Screen Shot 2020-10-23 at 3 57 01 PM

@aburgel
Copy link
Contributor Author

aburgel commented Oct 24, 2020

Just realized I didn't update all the scenario tests. There's a lot, so I'll hold off on that until there's some indication that this PR may be merged.

@P0lip
Copy link
Contributor

P0lip commented Nov 1, 2020

@philsturgeon what do you think about this one?

@philsturgeon
Copy link
Contributor

Do we want , in there? Is this just javascript turning an array into a string and defaulting to using , or is it intentional?

@aburgel
Copy link
Contributor Author

aburgel commented Nov 10, 2020

Do we want , in there? Is this just javascript turning an array into a string and defaulting to using , or is it intentional?

Yeah, it looks like it's an array getting turned into a string. I'm using IRuleResult.path which is of type JsonPath from @stoplight/types, which is an array of string | number.

Looks like there's a helper function that I can use: printPath. Any preference on the PrintStyle to use?

@aburgel
Copy link
Contributor Author

aburgel commented Nov 18, 2020

@philsturgeon @P0lip updated to use printPath

@@ -69,6 +70,7 @@ export const stylish: Formatter = results => {
getMessageType(result.severity),
result.code ?? '',
result.message,
printPath(result.path, PrintStyle.Dot),
Copy link
Contributor

Choose a reason for hiding this comment

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

This style is fine, I think. 👍

@P0lip P0lip added the enhancement New feature or request label Nov 20, 2020
@P0lip P0lip merged commit 83b1d61 into stoplightio:develop Nov 20, 2020
@aburgel aburgel deleted the aburgel/stylish-path branch November 20, 2020 13:48
@aburgel
Copy link
Contributor Author

aburgel commented Dec 12, 2020

@philsturgeon @P0lip what's the release process for spectral? i'm realizing now that this got merged into the develop branch, so it didn't get into the most recent release. when does develop get promoted to master and shipped?

i'd love to start using this feature.

@P0lip
Copy link
Contributor

P0lip commented Dec 14, 2020

@aburgel The change you implemented will be included in the 5.8.0 version. I'm planning to get it out before the end of 2020.

P0lip added a commit that referenced this pull request Jan 4, 2021
* feat: Add JSON path to stylish formatter

* Format with printPath

* Update test harness scenarios

Co-authored-by: Jakub Rożek <jakub@stoplight.io>
@P0lip
Copy link
Contributor

P0lip commented Jan 4, 2021

@aburgel it's in! Enjoy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants