Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Add L2 URI templates/fix issue on determination of varnames in URI templates #172

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dmartinezg
Copy link

@blakeembrey, @xaka, this brings L2 URI templates, and fixes an issue in the way the parser extracted the variable names from the templates.

Change is actually very minor, a couple of changes in validator and transformations, and 4 new tests.

'#%RAML 0.8',
'title: Title',
'baseUri: http://domain',
'/resource/{parameter1,parameter2}:'
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't level 2, I don't think we should accidentally test or support > 2.

Copy link
Author

Choose a reason for hiding this comment

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

That is an interesting point, yet, I don't know how to tell the URI template library not to accept these

Copy link
Contributor

Choose a reason for hiding this comment

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

Neither, can we just skip these tests and keep the implementation to level 2 only? The parser was already used for just level 1, so I'm guessing it's simple enough?

If it helps, other tooling I'll be implementing will literally be based on RegExps instead of the parser.

Copy link
Author

Choose a reason for hiding this comment

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

I refactored it

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any commit changes? Anyway, I'm currently support L2 parameters in https://github.com/mulesoft-labs/raml-path-match/blob/master/raml-path-match.js#L36 so you may be able to re-use that if you don't want the additional level handling.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants