-
Notifications
You must be signed in to change notification settings - Fork 153
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
Document limited uri-template support #955
Document limited uri-template support #955
Conversation
Signed-off-by: Matthias Pichler <m.pichler@warrify.com>
I'm not sure string substitution should be allowed everywhere, but instead should be restricted to URIs, else it would ask too much checks at runtime, don't you think? |
actually I think it should be available everywhere where a runtime expression could be used (so pretty much everywhere 😅 ). |
That's why I'd personally refrain from doing so, as you can easily do that using runtime expressions, which are much easier to spot for a runtime. This would imho become extremely cumbersome to have to check for both Furthermore, it would mean authors would not be able to use those characters without forcing runtimes to evaluate, even when not necessary, therefore needing some escaping feature. Anyhow, thats just me, let's see what @JBBianchi, @fjtirado and @ricardozanini think about it 😉 |
IMHO only simple substitution |
No strong opinion from my side. I am fine with only allowing this in URIs. In this case though I would rephrase the feature away from "string substitution" to "limited support for uri-template syntax". This would then allow us to expand to support additional uri-template features in the future (or now?). |
Signed-off-by: Matthias Pichler <m.pichler@warrify.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, aside from two little typos ;) Many thanks!
Signed-off-by: Matthias Pichler <m.pichler@warrify.com>
Although already changed, I agree with @cdavernas. String substitution should apply only to uri templates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks man!
Signed-off-by: Matthias Pichler m.pichler@warrify.com
Please specify parts of this PR update:
Discussion or Issue link:
As discussed in #910
What this PR does:
Documents the frequently used by undocumented string substitution feature
Additional information: