Skip to content

Commit

Permalink
Add the guessApproot function (pinging @gregwebs)
Browse files Browse the repository at this point in the history
  • Loading branch information
snoyberg committed Oct 13, 2015
1 parent e610f3b commit 5d0a456
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 3 deletions.
2 changes: 1 addition & 1 deletion yesod-auth/openid.hs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ $nothing
|]

instance Yesod BID where
approot = ApprootStatic "http://localhost:3000"
approot = guessApproot

instance YesodAuth BID where
type AuthId BID = Text
Expand Down
4 changes: 4 additions & 0 deletions yesod-core/ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.4.16

* Add `guessApproot`

## 1.4.15

* mkYesod avoids using reify when it isn't necessary. This avoids needing to define the site type below the call to mkYesod.
Expand Down
2 changes: 2 additions & 0 deletions yesod-core/Yesod/Core.hs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ module Yesod.Core
, MonadWidget (..)
, getRouteToParent
, defaultLayoutSub
-- * Approot
, guessApproot
-- * Misc
, yesodVersion
, yesodRender
Expand Down
17 changes: 17 additions & 0 deletions yesod-core/Yesod/Core/Class/Yesod.hs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import Yesod.Core.Internal.Session
import Yesod.Core.Widget
import Control.Monad.Trans.Class (lift)
import Data.CaseInsensitive (CI)
import qualified Network.Wai.Request

-- | Define settings for a Yesod applications. All methods have intelligent
-- defaults, and therefore no implementation is required.
Expand Down Expand Up @@ -826,3 +827,19 @@ fileLocationToString loc = (loc_package loc) ++ ':' : (loc_module loc) ++
where
line = show . fst . loc_start
char = show . snd . loc_start

-- | Guess the approot based on request headers. For more information, see
-- "Network.Wai.Middleware.Approot"
--
-- In the case of headers being unavailable, it falls back to 'ApprootRelative'
--
-- Since 1.4.16
guessApproot :: Approot site
guessApproot = ApprootRequest $ \_master req ->
case W.requestHeaderHost req of
Nothing -> ""
Just host ->
(if Network.Wai.Request.appearsSecure req
then "https://"
else "http://")
`T.append` TE.decodeUtf8With TEE.lenientDecode host
4 changes: 2 additions & 2 deletions yesod-core/yesod-core.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: yesod-core
version: 1.4.15
version: 1.4.16
license: MIT
license-file: LICENSE
author: Michael Snoyman <[email protected]>
Expand All @@ -25,7 +25,7 @@ library
build-depends: base >= 4.3 && < 5
, time >= 1.1.4
, wai >= 3.0
, wai-extra >= 3.0.5
, wai-extra >= 3.0.7
, bytestring >= 0.9.1.4
, text >= 0.7
, template-haskell
Expand Down

6 comments on commit 5d0a456

@paul-rouse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just wondering whether a generalisation is worthwhile: I have a situation where I have written almost exactly this, but with type (site -> Text) -> Approot site, making it fall back in effect to ApprootMaster. Or would the extra parameter be too obtrusive in other use cases?

@snoyberg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's funny, I'd actually been considering a guessApprootOr function with a signature just like that. I think there's room for both. To bikeshed slightly on the signature, what about:

(site -> request -> Text) -> Approot site

or

Approot site -> Approot site

so that no generality is lost.

@paul-rouse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be even better, and actually I like Approot site -> Approot site.

@snoyberg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to PR that as well? Should be straight-forward.

@paul-rouse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will do!

@snoyberg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

Please sign in to comment.