snapframework / snap-core

Core type definitions (Snap monad, HTTP types, etc) and utilities for web handlers.
http://snapframework.com/
BSD 3-Clause "New" or "Revised" License
317 stars 88 forks source link

snap-server: commandLineConfig does not allow overriding some default settings #65

Closed j3h closed 13 years ago

j3h commented 13 years ago

This is with snap-server 0.4.1. For instance, providing --hostname=foo does not change the hostname that gets used.

I think I traced this down to the mappend implementation for OptionData in Snap.Http.Server.Config:

a `mappend` b = OptionData
    { config       = (config       b) `mappend` (config       a)
    , bind         = (bind         b) `mplus`   (bind         a)
    , port         = (port         b) `mplus`   (port         a)
    , sslbind      = (sslbind      b) `mplus`   (sslbind      a)
    , sslport      = (sslport      b) `mplus`   (sslport      a)
    , sslcert      = (sslcert      b) `mplus`   (sslcert      a)
    , sslkey       = (sslkey       b) `mplus`   (sslkey       a)
    , tout         = (tout         b) `mplus`   (tout         a)
    }

In particular, the Monoid instance for Config prefers options from the second argument to mappend, so:

(config b) `mappend` (config a)

prefers options from config a, which is defaultConfig in the commandLineConfig case. I think the a and b need to be swapped in this expression.

dop commented 13 years ago

I should have created an issue for this one before pull request... Also, found two more bugs in handling of command line arguments and fixed them in my forked repository of snap-server dop/snap-server@6123d9076723c608fb455660f8a513c2f88c0953

gregorycollins commented 13 years ago

Should be fixed as of snap 0.5.0. Please re-open if not.