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

wai-app-static: honor ssIndices when used with defaultWebAppSettings #460

Merged
merged 5 commits into from Nov 10, 2015

Conversation

soenkehahn
Copy link
Contributor

See #376.

@soenkehahn soenkehahn changed the title wai-app-static: add test-case for ssIndices with defaultWebAppSettings) wai-app-static: honor ssIndices when used with defaultWebAppSettings Nov 5, 2015
@soenkehahn
Copy link
Contributor Author

I'm not sure if I prefer the version with or without the last commit (0c67261). @snoyberg: Any preference? Apart from that this is ready to merge from my side.

Any other feedback? Should I squash the commits?

@snoyberg
Copy link
Member

snoyberg commented Nov 6, 2015

I'm confused, this looks like a completely separate goal than #376. Can you clarify?

@soenkehahn
Copy link
Contributor Author

Sorry, I was also a bit confused.

If ssLookupFile returned an LRNotFound then wai-app-static assumed that the looked up file is not a directory and it didn't bother to further check for index files. ssIndices was only ever considered when ssLookupFile returned an LRFolder. The StaticSettings created with mkSettings would not return an LRFolder, which is the cause for #376.

Personally I ran into a similar issue, not with mkSettings but with defaultWebAppSettings. Its ssLookupFile also doesn't return LRFolders for performance reasons.

With my PR in the case that ssLookupFile returns LRFolder or LRNotFound ssLookupFile will be called again, this time searching for index files. So LRFolder will not be used for looking up index files anymore. This fixes both my issue and #376. I added a test case for #376.

@soenkehahn
Copy link
Contributor Author

The builds on travis are failing but it doesn't look like this is caused by my commits. Is travis for wai currently broken?

@snoyberg
Copy link
Member

snoyberg commented Nov 9, 2015

wai master is not broken. Can you try rebasing?

@soenkehahn
Copy link
Contributor Author

I rebased and added f088f32, now travis is happy. I hope it's the right way to fix this. (IIUC cabal-meta doesn't pick up test dependencies.)

snoyberg added a commit that referenced this pull request Nov 10, 2015
wai-app-static: honor ssIndices when used with defaultWebAppSettings
@snoyberg snoyberg merged commit 1bb6c53 into yesodweb:master Nov 10, 2015
@soenkehahn
Copy link
Contributor Author

Thanks for merging!

snoyberg added a commit that referenced this pull request Nov 11, 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.

None yet

2 participants