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

Docopt for CLI parsing #27

Merged
merged 2 commits into from
Mar 7, 2019
Merged

Docopt for CLI parsing #27

merged 2 commits into from
Mar 7, 2019

Conversation

Anviking
Copy link
Collaborator

@Anviking Anviking commented Mar 4, 2019

Issue Number

#7

Overview

  • I have tried out docopt for parsing command line arguments

Comments

Usage:

All options have default values

$ stack build
$ stack exec cardano-wallet-server
TODO: start wallet on port 8090,
      connecting to Mainnet node on port 8080

$ stack exec cardano-wallet-server --
TODO: start wallet on port 8090,
      connecting to Mainnet node on port 8080

$ stack exec cardano-wallet-server -- --wallet-server-port 2
TODO: start wallet on port 2,
      connecting to Mainnet node on port 8080

$ stack exec cardano-wallet-server -- --network testnet
TODO: start wallet on port 8090,
      connecting to Testnet node on port 8080

Help

$ stack exec cardano-wallet-server -- help
cardano-wallet-server

Start the cardano wallet along with its API and underlying node.

Usage:
  cardano-wallet-server [options]
  cardano-wallet-server --help

Options:
  --wallet-server-port <PORT>  port used for serving the wallet API [default: 8090]
  --node-port <PORT>           port used for node-wallet communication [default: 8080]
  --network <NETWORK>          mainnet or testnet [default: mainnet]

Errors

$ stack exec cardano-wallet-server -- --network afs
Invalid LongOption "network". "afs" is neither "mainnet" nor "testnet".

cardano-wallet-server

Start the cardano wallet along with its API and underlying node.

Usage:
  cardano-wallet-server [options]
  cardano-wallet-server --help

Options:
  --wallet-server-port <PORT>  port used for serving the wallet API [default: 8090]
  --node-port <PORT>           port used for node-wallet communication [default: 8080]
  --network <NETWORK>          mainnet or testnet [default: mainnet]


$ stack exec cardano-wallet-server -- --wallet-server-port a
Invalid LongOption "wallet-server-port". Not an integer: "a".

cardano-wallet-server

Start the cardano wallet along with its API and underlying node.

Usage:
  cardano-wallet-server [options]
  cardano-wallet-server --help

Options:
  --wallet-server-port <PORT>  port used for serving the wallet API [default: 8090]
  --node-port <PORT>           port used for node-wallet communication [default: 8080]
  --network <NETWORK>          mainnet or testnet [default: mainnet]

@Anviking Anviking self-assigned this Mar 4, 2019
--wallet-server-port Number of core nodes to start [default: 4]
--node-port Number of relay nodes to start [default: 1]
--network mainnet or testnet [default: mainnet]
|]
Copy link
Member

Choose a reason for hiding this comment

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

Note that we could use docoptFile here and point to a file in specifications/launcher

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Isn't it nice to have it here, in the same place?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Maybe we can just have a symlink or something that points to this particular file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a readme file now at specifications/CLI.md (wrong place?)

About symlink: In that case we would have to make sure the module level comment targets both developers and users… which might arguably be a good thing.

I could try that.

@KtorZ KtorZ mentioned this pull request Mar 6, 2019
1 task
@Anviking Anviking changed the title Experiment: docopt for CLI parsing Docopt for CLI parsing Mar 6, 2019
@Anviking Anviking force-pushed the anviking/7/docopt branch 4 times, most recently from f840906 to 8b69d4e Compare March 6, 2019 11:48
" node on port " ++ (show nodePort)
where
readInt :: String -> Int
readInt s = maybe (error $ "not an integer: " ++ show s) id (readMaybe s)
Copy link
Collaborator Author

@Anviking Anviking Mar 6, 2019

Choose a reason for hiding this comment

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

I guess we could do something more proper and testable for this, by having everything in

:: [String] -> Either ErrorOutput a

or some ReaderT monad, or something. Haven't tried yet.

@Anviking Anviking requested a review from KtorZ March 6, 2019 11:56
--wallet-server-port Number of core nodes to start [default: 4]
--node-port Number of relay nodes to start [default: 1]
--network mainnet or testnet [default: mainnet]
|]
Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Maybe we can just have a symlink or something that points to this particular file.

app/server/Main.hs Show resolved Hide resolved
app/server/Main.hs Show resolved Hide resolved
app/server/Main.hs Outdated Show resolved Hide resolved
app/server/Main.hs Outdated Show resolved Hide resolved
app/server/Main.hs Show resolved Hide resolved
app/server/Main.hs Outdated Show resolved Hide resolved
@Anviking Anviking force-pushed the anviking/7/docopt branch 2 times, most recently from b1d0db6 to 3bda6e5 Compare March 7, 2019 09:01
readNetwork :: String -> Either String Network
readNetwork "mainnet" = Right Mainnet
readNetwork "testnet" = Right Testnet
readNetwork s = Left $ show s ++ " is neither \"mainnet\" nor \"testnet\"."
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't want to complicate things by not hardcoding this message.

case decode str of
Right a -> return a
Left err -> do
putStrLn $ "Invalid " <> show opt <> ". " <> err
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed some syntactical structure from your example. Now it can read like

Invalid LongOption "wallet-server-port". Not an integer: "a".

" node on port " ++ (show nodePort)


-- Functions for parsing the values of command line options
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't really matter here because executable don't have corresponding haddock documentation, but ideally, documentation for functions starts with -- |, otherwise, the comment is just a plain comment and is ignored during generation.

Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

Fantastic PR's comments ❤️

@KtorZ KtorZ merged commit 70dc6af into master Mar 7, 2019
@KtorZ KtorZ deleted the anviking/7/docopt branch March 7, 2019 10:32
KtorZ added a commit that referenced this pull request Mar 11, 2019
paweljakubas added a commit that referenced this pull request Mar 11, 2019
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes
paweljakubas added a commit that referenced this pull request Mar 11, 2019
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes

[3] hlint suggestion
paweljakubas added a commit that referenced this pull request Mar 11, 2019
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes

[3] hlint suggestion

[3] add .weeder.yaml to omit duplicateMaybes and groups to be announce during weeder execution

[3] undo weeder ignore file plus remove pragma and unnecessary exports
paweljakubas added a commit that referenced this pull request Mar 11, 2019
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes

[3] hlint suggestion

[3] add .weeder.yaml to omit duplicateMaybes and groups to be announce during weeder execution

[3] undo weeder ignore file plus remove pragma and unnecessary exports

[3] remove not needed imports
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants