Skip to content

Adding support for tabs along with spaces for inline comments#252

Merged
erunion merged 1 commit intoreadmeio:mainfrom
garrett-wade:tabSupport
Mar 17, 2022
Merged

Adding support for tabs along with spaces for inline comments#252
erunion merged 1 commit intoreadmeio:mainfrom
garrett-wade:tabSupport

Conversation

@garrett-wade
Copy link
Copy Markdown
Contributor

@garrett-wade garrett-wade commented Mar 17, 2022

This is an initial enhancement for the this issue.

This enhancement

  • replaces \t characters with four spaces prior to loading the yaml the js-yaml library.

Testing

Create a file with an inline @oas comment and use tabs instead of spaces when specifying the underlying yaml and run a swagger-inline command on that file.

- replace \t characters with four spaces when using the` jsYaml.load()` method
@garrett-wade
Copy link
Copy Markdown
Contributor Author

@erunion understand that this does not replace a /t instances any thoughts here? Have also tried jsYaml.load(yamlLines.join('\n').replace(/\t/gy,' ')); with no luck

@erunion
Copy link
Copy Markdown
Member

erunion commented Mar 17, 2022

@garrett-wade Not sure I'm familiar /t instances, but this work looks ok to me.

@erunion erunion merged commit c446cc9 into readmeio:main Mar 17, 2022
@garrett-wade
Copy link
Copy Markdown
Contributor Author

@erunion ran some more testing and looks like it needs to be return jsYaml.load(yamlLines.join('\n').replace(/\t/g,' ')); as this PR only support replace a single /t and /t is a tab in a string.

@garrett-wade garrett-wade deleted the tabSupport branch March 17, 2022 22:05
@erunion
Copy link
Copy Markdown
Member

erunion commented Mar 17, 2022

k, I'll add that in a follow up commit

@erunion
Copy link
Copy Markdown
Member

erunion commented Mar 17, 2022

Published to 5.1.1. Thanks! https://github.com/readmeio/swagger-inline/releases/tag/5.1.1

@garrett-wade
Copy link
Copy Markdown
Contributor Author

@erunion just double checking, are you going to make the update or should I do another PR? Thanks for the timely responses here too.

@erunion
Copy link
Copy Markdown
Member

erunion commented Mar 18, 2022

@garrett-wade i already did and it's in that release! 🙂

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.

2 participants