Skip to content

Append slash on directories - #33

Open
connyay wants to merge 2 commits into
expressjs:masterfrom
connyay:append-slash
Open

Append slash on directories#33
connyay wants to merge 2 commits into
expressjs:masterfrom
connyay:append-slash

Conversation

@connyay

@connyay connyay commented Jun 30, 2015

Copy link
Copy Markdown

No description provided.

@connyay

connyay commented Jun 30, 2015

Copy link
Copy Markdown
Author

I should have looked at the other PRs... looks like this has come up quite a bit. I guess I should close this in favor of #31

@dougwilson

Copy link
Copy Markdown
Contributor

Hi! Have you taken a look at the other two PRs for this yet? Can you comment on the difference between your PR and those?

@dougwilson

Copy link
Copy Markdown
Contributor

lol, sorry, looks like we were racing to type first :)

@connyay

connyay commented Jun 30, 2015

Copy link
Copy Markdown
Author

No worries. It was my bad.

This PR is pretty lacking. I should have looked at json & plain closer.

Are you still looking for: #31 (comment) ? Do you think trailingSlashes should be an option? To match apache & nginx, I personally think that this should be the default.

Katello (Red Hat Satellite) uses this code to crawl a directory listing and find necessary files. I would assume there are probably some other crawlers that work in a similar way.

I can work on unifying #27 & #31 (& now this one). Just let me know which one you are leaning toward.

Thanks!

@dougwilson

Copy link
Copy Markdown
Contributor

Awesome. Let me collect my thoughts for you and then when I get home in a bit, I'll write them out :)

As for it being an option, unfortunately we did a while ago try just adding trailing slashes but it broke people. I 100% agree it should be default, but to do that we do need to major bump based on past problems. We are open for a major bump Aug 1 if you want to wait, otherwise we can add it as optional now and swap the default on the Aug 1 major.

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