Skip to content

Conversation

@piranna
Copy link
Contributor

@piranna piranna commented Dec 9, 2015

Allow to define a function to generate the content of directory indexes

@dougwilson dougwilson self-assigned this Dec 10, 2015
@dougwilson dougwilson added the pr label Dec 10, 2015
@dougwilson
Copy link
Contributor

Hi again, @piranna ! Thanks for this pull request! Currently, this module sports an event emitter-driven interface (for better or worse). With the existing interface, this functionality already exists and is well supported (in fact, is being done on other modules on npm). Adding yet another way to do the same thing, especially where the interface is oddly different from the rest of the module, would be weird to accept.

I would recommend you use the existing functionality at hand. If you have suggestion on making the EE interface better, it would be welcome! Here is the functionality from your test done using the existing interface (your test is the only communication we've had in regards to this, so that's the only example I have from you to use):

send(req, req.url, {index: false})
.on('directory', function () {
  // send your index from here, as the req.url provided ended up being a directory
  this.res.end('<p>tobi</p>');
})
.pipe(res);

@dougwilson dougwilson closed this Dec 10, 2015
@piranna
Copy link
Contributor Author

piranna commented Dec 10, 2015

Hi @dougwilson :-) I have review the code (again ;-) ) and the directory event is only emited when the "file" we are trying to access is in fact a directory, and the current behaviour is to redirect it the slash-ended path, that I think is a good feature that I would loose by attaching to the directory event. In this case, what we would need is an index event, this way we could have the redirection and the custom index rendering.

I'm totally +1 to the event-driven APIs, I'll update the pull-request to add an index event and use it :-)

@piranna
Copy link
Contributor Author

piranna commented Dec 10, 2015

Seems GitHub don't allow to push new commits to a pull-request if it's clossed, so I've open a new one with the index event.

Just a question: shouldn't this and the directory be better managed with a callback on the constructor? There should be only just one function to render them... This way the current behaviour would be enclosed in their own functions and set as default ones, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants