Use optparse-applicative instead of cmdargs #111
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dahlia
requested changes
Mar 11, 2017
nirum.cabal
Outdated
| @@ -133,6 +134,7 @@ test-suite spec | |||
| , megaparsec | |||
| , mtl | |||
| , nirum | |||
| , optparse-applicative >=0.13.1 && <0.14 | |||
There was a problem hiding this comment.
test-suite spec 쪽은 library 쪽 의존성 버전을 따라가면 되니까, 버전 명시 안해도 될 것 같습니다.
src/Nirum/Cli.hs
Outdated
| optsParser = | ||
| OPT.info | ||
| (OPT.helper <*> versionOption <*> programOptions) | ||
| (OPT.fullDesc <> OPT.progDesc "Nirum compiler." <> |
There was a problem hiding this comment.
원래는 버전도 같이 표시했었는데 빠졌네요. 마침표는 없어도 될 것 같습니다.
src/Nirum/Cli.hs
Outdated
| programOptions :: OPT.Parser Opts | ||
| programOptions = | ||
| Opts <$> OPT.strOption | ||
| (OPT.long "outdir" <> OPT.short 'o' <> OPT.metavar "OUTDIR" <> |
There was a problem hiding this comment.
이거 metavar은 DIR이면 될 것 같습니다. Long option name은 output-dir 정도로 푸는 게 나아 보입니다.
src/Nirum/Cli.hs
Outdated
| OPT.help "out directory") <*> | ||
| OPT.strOption | ||
| (OPT.long "target" <> OPT.short 't' <> OPT.metavar "TARGET" <> | ||
| OPT.help "Target name") <*> |
There was a problem hiding this comment.
설명은 “target language name” 정도로 적어주세요.
src/Nirum/Cli.hs
Outdated
| (OPT.long "target" <> OPT.short 't' <> OPT.metavar "TARGET" <> | ||
| OPT.help "Target name") <*> | ||
| OPT.strArgument | ||
| (OPT.metavar "PACKAGE" <> OPT.help "Package directory") |
There was a problem hiding this comment.
마찬가지로 metavar은 DIR 정도가 적당할 것 같습니다.
dahlia
requested changes
Mar 11, 2017
src/Nirum/Cli.hs
Outdated
| , (&=) | ||
| ) | ||
| import System.Console.CmdArgs.Default (def) | ||
| import Data.Monoid |
There was a problem hiding this comment.
이 모듈에서 어떤 거 가져다 쓰는 거예요? import qualified로 하거나 필요한 것만 명시적으로 가져오는 게 좋을 듯하네요.
dahlia
requested changes
Mar 11, 2017
README.md
Outdated
| -V --version Print version information | ||
| --numeric-version Print just the version number | ||
| Usage: nirum [-v|--version] (-o|--output-dir DIR) (-t|--target TARGET) DIR | ||
| Nirum compiler0.3.0 |
README.md
Outdated
| @@ -55,17 +55,17 @@ In order to compile a Nirum package (`examples/`) to a Python package: | |||
| For more infomration, use `--help` option: | |||
|
|
|||
| $ nirum --help | |||
| Nirum Compiler 0.3.0 | |||
| nirum - The IDL compiler and RPC/distributed object framework | |||
dahlia
approved these changes
Mar 11, 2017
yjroot
approved these changes
Mar 11, 2017
dahlia
added a commit
that referenced
this pull request
Mar 13, 2017
Fix regressive things PR #111 missed
dahlia
added
the
cmp:frontend
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use optparse-applicative which one is suggested in #88. this PR close #88 as well.