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

Add command line Arguments #5

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add command line Arguments #5

wants to merge 10 commits into from

Conversation

tobhe
Copy link
Contributor

@tobhe tobhe commented Mar 23, 2016

Reader Monad

To be able to overwrite config settings with command line arguments i had to rethink the way we access the config file.
In the new solution i removed the now unnecessary getter functions like getPort :: IO Int
and instead made the Config globally accessible by using the monad transformer ReaderT and keeping the config in the Reader Monad.
Therefore all functions needing config acces now have a type signature of:

(MonadReader Config m, MonadIO m) => a -> m (a)

The config itself can now be loaded with Readers ask function.
Single config records can be accessed via asks as in myVersion <- asks acceptedVersion

Command line arguments

Command line arguments are parsed in a new File calles Args.hs.
it provides a function named parseOpts which if successful returns a Config object as loaded from the config file, where specially specified records like port are overwritten with the command line arguments.
Usage info can be printed with -h or --help and looks like this:

Usage: openage-masterserver [options]...
Options:

  -h       --help         Print help
  -c FILE  --config=FILE  config file to use
  -p NUM   --port=NUM     port to use


# Local packages, usually specified by relative directory name
packages:
- '.'
# Packages to be pulled from upstream that are not in the resolver (e.g., acme-missiles-0.3)
extra-deps: []
extra-deps: [aeson-0.11.0.0]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TheJJ This is btw necessary for using aeson >= 0.11, because lts-5.9 provides aeson-0.9 by default and stack won't be able to pull in dependencies otherwise

Copy link
Member

Choose a reason for hiding this comment

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

kay, but if you write it that way, sure it's >=?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no it's actually 0.11.0.0, and as far as i know there's no way of saying >=0.11 in extra deps.
But i know that one is working for now and higher versions like 0.11.1.3 gave me some dependency conflict.
Edit: what i meant above was for using aeson >= 0.11 in the cabal file

Copy link
Member

Choose a reason for hiding this comment

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

Mkay but if you have a dependency conflict then it's not an issue of the masterserver. Stack surely supports some notation for having a minimal version of some package, setting it to "we require exactly 0.11.0.0" is really not good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, the cleanest solution would be not using >=0.11 while it's not in the long term support package, a second possible way but still not too good would be using a nightly package set instead of lts, like this, which actually features aeson-0.11.1.4, but still needs to be updated with every new release.

Copy link
Member

Choose a reason for hiding this comment

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

Why does stack suck and not support a dependency specification with >=, I mean we now do require at least aeson 0.11, stack must be able to represent that somehow.

Copy link
Member

@TheJJ TheJJ left a comment

Choose a reason for hiding this comment

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

Pls fix conflict in stack.yml. Should work now with newer aeson through #8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants