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

Allow subsites within hierarchical routes #1144

Merged
merged 2 commits into from Jan 14, 2016

Conversation

Projects
None yet
2 participants
@ajnsit
Copy link
Contributor

ajnsit commented Jan 13, 2016

This allows subsites to work within hierarchical routes like so -

/sub SubR:
  /hello HelloSubR HelloSubRoute getHelloSubRoute

HelloSubR takes the subsite route as an argument which needs to be folded through the parent route constructors. Not doing so is definitely a bug, regardless of whether hierarchical subsites are supported.

@snoyberg

This comment has been minimized.

Copy link
Member

snoyberg commented Jan 13, 2016

Can you add a test case of something that didn't work previously and does work now?

@ajnsit

This comment has been minimized.

Copy link
Contributor Author

ajnsit commented Jan 13, 2016

Do you mean a testcase in the test suite for yesod? I tried adding a test case but I couldn't immediately figure out how the test suite works. Does the test suite currently test any subsite functionality? I can use that as a template and try again when I have some more time.

Meanwhile, I have created a small yesod project to demonstrates the problem (https://github.com/ajnsit/yesod-subsite-test). If I try to compile this project without this PR, I get error messages (https://github.com/ajnsit/yesod-subsite-test/blob/master/error_master.txt).

Looking at the error message - It tries to assign the type Route HelloSub -> Route App to SubR SubsiteNotOkR. With this PR, the TH will generate SubR . SubsiteNotOkR instead which has the correct type.

@snoyberg

This comment has been minimized.

Copy link
Member

snoyberg commented Jan 13, 2016

Yes, I meant a yesod-core test suite case. You can look in
test/YesodCoreTest for some simple mostly-standalone tests. WaiSubsite, for
example, includes a subsite in it, which may be a good template.

On Wed, Jan 13, 2016 at 10:02 AM, Anupam Jain notifications@github.com
wrote:

Do you mean a testcase in the test suite for yesod? I tried adding a test
case but I couldn't immediately figure out how the test suite works. Does
the test suite currently test any subsite functionality? I can use that as
a template and try again when I have some more time.

Meanwhile, I have created a small yesod project to demonstrates the
problem (https://github.com/ajnsit/yesod-subsite-test). If I try to
compile this project without this PR, I get error messages (
https://github.com/ajnsit/yesod-subsite-test/blob/master/error_master.txt
).

Looking at the error message - It tries to assign the type Route HelloSub
-> Route App to SubR SubsiteNotOkR. With this PR, the TH will generate SubR
. SubsiteNotOkR instead which has the correct type.


Reply to this email directly or view it on GitHub
#1144 (comment).

@ajnsit

This comment has been minimized.

Copy link
Contributor Author

ajnsit commented Jan 14, 2016

Done! Thanks for the pointer to WaiSubsite! It was extremely easy to add a test case to that.

snoyberg added a commit that referenced this pull request Jan 14, 2016

Merge pull request #1144 from ajnsit/hierarchical-subsites
Allow subsites within hierarchical routes

@snoyberg snoyberg merged commit 8f2d92b into yesodweb:master Jan 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

snoyberg added a commit that referenced this pull request Jan 14, 2016

@snoyberg

This comment has been minimized.

Copy link
Member

snoyberg commented Jan 14, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.