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

Vatican.requestHandler #23

Open
smart--petea opened this issue Sep 20, 2014 · 4 comments
Open

Vatican.requestHandler #23

smart--petea opened this issue Sep 20, 2014 · 4 comments
Labels

Comments

@smart--petea
Copy link
Contributor

In Vatican.requestHandler there is the snippet

var hdlr = this.loadHandler(process.cwd() + "/" + methodFound.handlerPath)

If options.handlers is defined as relative path all it's ok. If options.handlers is defined as absolute paths the methodFound.handlerPath is an absolute path and the expression process.cwd() + "/" + methodFound.handlerPath will give something like

/home/ppp//home/ppp/tt/dd

plus an error of reading the directory.

Maybe

  1. to import path module
  2. rewrite function Vatican.parseHandlers
Vatican.prototype.parseHandlers = function(cb) {
  ...
  var dir = path.isAbsolute(this.options.handlers) ? this.options.handlers : process.cwd() + "/" + this.options.handlers;
  ...
}
  1. rewrite Vatican.requestHandler
Vatican.prototype.requestHandler = function(req, res) {
...
var hdlr = this.loadHandler( methodFound.handlerPath)
...
}
@deleteman
Copy link
Owner

@smart--petea the method "isAbsolute" does not seem to be part of the "path" module: http://nodejs.org/api/path.html
How is it working for you?

@smart--petea
Copy link
Contributor Author

I'm working with experimental node with support for generators (version 0.11.*). But version 0.10.32 (the current stable version) support the isAbsolute function.

@deleteman
Copy link
Owner

@smart--petea please make sure you don't add code from experimental versions of Node on your PRs.
Also, I don't see the support for isAbsolute method in version 0.10.32, and even it if was supported, it's not supported by version 0.10.22 (which is what I have) and I don't want to force people to update their node.js to the latest version in order for Vatican to actually work. So please, use a different method.

Thanks

@smart--petea
Copy link
Contributor Author

You are right. I misunderstood their source code from version 0.10.32 - https://github.com/joyent/node/blob/v0.10.32-release/lib/path.js. There they use isAbsolute in quality of simple variable.

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

No branches or pull requests

2 participants