Skip to content

Conversation

@jemerald
Copy link

@jemerald jemerald commented Feb 11, 2018

Description

Escape JSON path for definitions when resolving auto-suggest and when performing validation.

Extracted the escapeJsonPointerToken and unescapeJsonPointerToken functions to separate file.

Also added cross-env dev dependency to allow running the test under windows.

Motivation and Context

Fixes #1644 and Fixes #1645

How Has This Been Tested?

Manually tested auto-completion in Chrome.

Added unit test for validation.

Screenshots (if appropriate):

image

image

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@@ -1,5 +1,6 @@
import { createSelector } from "reselect"
import { Set, Map } from "immutable"
import { escapeJsonPointerToken } from "../refs-util"
Copy link
Author

@jemerald jemerald Feb 11, 2018

Choose a reason for hiding this comment

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

@shockey Is this the preferred way for sharing code? Or should it go through the magic plugin system?

Copy link

Choose a reason for hiding this comment

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

This seems fine to me - the plugin system is great for exposing code that someone else may want to override/customize, but I can't imagine someone wanting to write a custom JSON Pointer escaper 😄

Copy link

@shockey shockey left a comment

Choose a reason for hiding this comment

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

LGTM!

@shockey shockey merged commit 1879983 into swagger-api:master Feb 27, 2018
@shockey
Copy link

shockey commented Feb 27, 2018

thanks, @jemerald!

@jemerald jemerald deleted the escape-json-ref branch February 27, 2018 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bogus "unused definition" warnings if schema names contain / or ~ $ref autocompletion for definitions whose names contain / or ~

2 participants