fix: handling letter cases when merging http ops #83
Conversation
karol-maciaszek
requested review from
lottamus,
marbemac and
XVincentX
as code owners
src/merge.ts
Outdated
| @@ -20,12 +20,12 @@ export const mergeResponses = mergeLists<IHttpOperationResponse>( | |||
| ); | |||
|
|
|||
| const mergeServers = mergeLists<IServer>( | |||
| (s1, s2) => s1.url === s2.url, | |||
| (s1, s2) => s1.url.toLowerCase() === s2.url.toLowerCase(), | |||
There was a problem hiding this comment.
Actually @philsturgeon you could give your idea on this too.
There was a problem hiding this comment.
I don’t know how many people are doing this in the wild but yeah technically they could. Let’s leave this out and see if it’s reported, either by reports we can run or by users.
There was a problem hiding this comment.
I just removed it.
I believe we should have it like that: case-insensitive host and case-sensitive path. WDYT?
There was a problem hiding this comment.
I agree, HOSTS should be case insensitive. It this URL a full one (comprehensive of the host)?
There was a problem hiding this comment.
Well, that is even funnier. See the typings: https://github.com/stoplightio/types/blob/master/src/servers.ts#L8
We can have variables inside the server URL. I'll check this out, but it seems that we need to parse the URL in order to understand what is case-(in)?sensitive.
- host name is case-insensitive
- URL is case-sensitive
- variable name: I can't find in OAS spec whether this is case sensitive or not, but I would assume it is
Now the funny part: a variable inside hostname makes it partially case-sensitive.
I guess we need to address it now or create an issue to address it later. Actually I'm also thinking about http-spec being responsible for server.url sanitization while transforming from different spec formats.
There was a problem hiding this comment.
Yeah let's make a separate issue for this, we can get this over the finish line. Feel free to open an issue.
There was a problem hiding this comment.
Here is the task: #84
|
🎉 This PR is included in version 2.8.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Case insensitive comparison of header/mediaType/server.url when merging.