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

[REST] Allow empty path to be treated as '/' #1135

Merged
merged 3 commits into from
Jun 15, 2015

Conversation

Geod24
Copy link
Contributor

@Geod24 Geod24 commented Jun 13, 2015

The first steps taken are to assert on an empty / null path in the router.

I then considered a special handling for empty string as path, which would basically make it work like '/'.
But that would be quite an annoying change, but it could be potentially dangerous on update. So there's now an assert in vibe.web.common.extractHTTPMethodAndName.
In addition, I realized that the web interface was using it, so I made the error message clearer. The change involved is quite trivial.

Note about unrelated changes: I (finally) set up editor config on my emacs. Which means everytime I save a file I edited, it is normalized to be conformant to the settings in .editorconfig, which is why there is so much noise.

@s-ludwig
Copy link
Member

Disallowing @path("") sounds fine, but why do you also want to remove get() as GET /? This has been in use almost from the beginning on and would potentially break a lot of code for no gain. It should be allowed for both, the web and the REST interface generators.

@Geod24 Geod24 force-pushed the userman-rest branch 2 times, most recently from 8d5e92e to 71be094 Compare June 13, 2015 17:22
Geod24 added 3 commits June 13, 2015 19:24
Previously, having a function named with one of the prefixes
(e.g: `get`, `post`, `query`) would result in a different
behavior in the web and rest interface. The REST interface
will generate faulty code that could crash at some point,
while the web generator will treat that as a '/'.

Instead of special-casing this, this commit makes it an error.
This error will be triggered when `registerXXXInterface` is
called, thus it is a CT error, but it was not possible to
make it a deprecation since we're dealing with CTFE.
@Geod24
Copy link
Contributor Author

Geod24 commented Jun 13, 2015

Updated to transform "" into "/"`.

@Geod24 Geod24 changed the title Remove an inconsistency between vibe.web.web and vibe.web.rest [REST] Allow empty path to be treated as '/' Jun 13, 2015
@Geod24 Geod24 mentioned this pull request Jun 13, 2015
s-ludwig added a commit that referenced this pull request Jun 15, 2015
[REST] Allow empty path to be treated as '/'
@s-ludwig s-ludwig merged commit d59cb73 into vibe-d:master Jun 15, 2015
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.

3 participants