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

Use + as marker for path parameters when splitting a specification #1336

Open
meden opened this issue Nov 20, 2023 · 10 comments
Open

Use + as marker for path parameters when splitting a specification #1336

meden opened this issue Nov 20, 2023 · 10 comments

Comments

@meden
Copy link

meden commented Nov 20, 2023

Describe the bug

When splitting an existing specification, and individual path files are generated, the parameters in path are enclosed in {} in the names of corresponding generated path files.

The characters { and } need escaping when referred in Linux shell, and this makes harder to manage them.

To Reproduce
Steps to reproduce the behavior:

  1. Given any working redocly.yaml file
  2. Given any valid openapi-bundle.yaml file having at least one endpoint with path parameters
  3. Run openapi split openapi-bundle.yaml --outDir ./split
  4. Try to refer a path file relative to an endpoint with a path parameter: it will require something like paths/book_\{bookId\}.yaml

Expected behavior

It would be preferable to have filenames without special characters in their names.

A good candidate seems to be #+, as it looks like it is not needing any escaping in Windows. To preserve retro-compatibility, the marker character could eventually be configurable.

Redocly Version(s)

1.0.0-beta.125

Node.js Version(s)

v18.13.0

@meden meden added the Type: Bug Something isn't working label Nov 20, 2023
@meden
Copy link
Author

meden commented Nov 20, 2023

Spoke too soon... The # is indeed reserved in references. + could be an alternative.

@meden meden closed this as completed Nov 20, 2023
@meden meden reopened this Nov 20, 2023
@meden meden changed the title Use # as marker for path parameters when splitting a specification Use + as marker for path parameters when splitting a specification Nov 20, 2023
@tatomyr
Copy link
Contributor

tatomyr commented Nov 23, 2023

Hi @meden,
I don't think this is a bug, rather an enhancement 🙂
I wouldn't change the default behaviour but allow to configure the characters with some option or through the config file. Not sure about the syntax though.

@tatomyr tatomyr added p3 Type: Enhancement and removed Type: Bug Something isn't working labels Nov 23, 2023
@tatomyr
Copy link
Contributor

tatomyr commented Nov 23, 2023

Note: This might be tangentially related to #1285.

@meden
Copy link
Author

meden commented Nov 27, 2023

I don't think this is a bug, rather an enhancement

@tatomyr definitely, sorry for using the wrong template.

I wouldn't change the default behaviour but allow to configure the characters with some option or through the config file. Not sure about the syntax though.

Indeed it would be the "safest" and most flexible solution. A possible way to do it could be something like a parameter:

--paramEnclosing=<openingChar><closingChar>

E.g. the current behavior would correspond to the parameter:

--paramEnclosing={}

Having no enclosing characters should also be allowed.

@adamaltman
Copy link
Member

Try to refer a path file relative to an endpoint with a path parameter: it will require something like paths/book_{bookId}.yaml

When you say "refer" do you mean using $ref or something else? Because our $ref resolver works fine on Linux without needing to escape.

Here is a working example:

https://github.com/Rebilly/api-definitions/blob/main/openapi/openapi.yaml#L621-L622

@meden
Copy link
Author

meden commented Dec 12, 2023

$ref resolver works fine on Linux without needing to escape.

I'm not talking about escaping in YAML, but referring the file in a shell (e.g. by hitting TAB to auto-complete its name).

I'm asking to have a way to produce file names without special characters.

BTW, at least with this IntelliJ IDEA plugin, using curly braces in $refs causes parsing issues nonetheless, so the requested ability is needed also to prevent this.

image

@thesteve0
Copy link

Ran into this issue as well. I actually do think it is a bug. The CLI is creating invalid URIs. I spent quite a bit of time

  1. Figuring out this was the issue
  2. Deciding how to fix it
  3. Then fixing it - requires both changing the file name and the ref in the OpenAPI.json.

Default behavior should produce valid URIs and then let people change it to { } if that's what they want.

@thesteve0
Copy link

~ seems to be a good character and it is infrequently used in text

https://www.rfc-editor.org/rfc/rfc3986#page-13

@lornajane
Copy link
Collaborator

Changing the default behaviour is probably not an option until the next major release, since it will break a large number of existing workflows. However I think either + or ~ should be considered as an optional parameter to use so that we can make this functionality available sooner.

@thesteve0
Copy link

thesteve0 commented Jun 4, 2024 via email

@lornajane lornajane added the split Split command label Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants