ndmitchell / cmdargs

Haskell library for command line argument processing
Other
91 stars 12 forks source link

Some arguments aren't grouped under 'Common Flags' (cmdargs 0.10.7) #3

Closed dan-t closed 10 years ago

dan-t commented 10 years ago

Hi Neil,

some arguments seem to be considered, others not, here -e, -l, -t and -b aren't considered:

dan@machine ~> cabal-bounds --help
cabal-bounds [COMMAND] ... [OPTIONS]
  A command line program for managing the bounds/versions of the dependencies
  in a cabal file.

Common flags:
  -O --only=ITEM             Only the bounds of the dependency are modified.
  -I --ignore=ITEM           This dependency is ignored, not modified in any
                             way.
  -o --outputCabalFile=ITEM  Save modified cabal file to file, if empty, the
                             cabal file is modified inplace.
  -h --help                  Display help message
  -v --version               Print version information

cabal-bounds drop [OPTIONS] CABAL-FILE

  -U --upper                 Only the upper bound is dropped, otherwise both
                             - the lower and upper - bounds are dropped.
  -l --library               Only the bounds of the library are dropped.
  -e --executable=ITEM       Only the bounds of the executable are dropped.
  -t --testsuite=ITEM        Only the bounds of the test suite are dropped.
  -b --benchmark=ITEM        Only the bounds of the benchmark are dropped.

cabal-bounds update [OPTIONS] CABAL-FILE SETUP-CONFIG-FILE

  -L --lower                 Only the lower bound is updated.
  -U --upper                 Only the upper bound is updated.
  -l --library               Only the bounds of the library are updated.
  -e --executable=ITEM       Only the bounds of the executable are updated.
  -t --testsuite=ITEM        Only the bounds of the test suite are updated.
  -b --benchmark=ITEM        Only the bounds of the benchmark are updated.

The data structure for the arguments is:

data Args = Drop { upper           :: Bool
             , library         :: Bool
             , executable      :: [String]
             , testSuite       :: [String]
             , benchmark       :: [String]
             , only            :: [String]
             , ignore          :: [String]
             , outputCabalFile :: String
             , cabalFile       :: String
             }
      | Update { lower           :: Bool
               , upper           :: Bool
               , library         :: Bool
               , executable      :: [String]
               , testSuite       :: [String]
               , benchmark       :: [String]
               , only            :: [String]
               , ignore          :: [String]
               , outputCabalFile :: String
               , cabalFile       :: String
               , setupConfigFile :: String
               } 
      deriving (Data, Typeable, Show, Eq)

And the initialization of the arguments:

get :: IO Args
get = cmdArgsRun . cmdArgsMode $ modes [dropArgs, updateArgs]
   &= program "cabal-bounds"
   &= summary summaryInfo
   &= help "A command line program for managing the bounds/versions of the dependencies in a cabal file."
   &= helpArg [explicit, name "help", name "h"]
   &= versionArg [explicit, name "version", name "v", summary versionInfo]
   where
      summaryInfo = ""

dropArgs :: Args
dropArgs = Drop
   { upper           = def &= explicit &= name "upper" &= name "U" &= help "Only the upper bound is dropped, otherwise both - the lower and upper - bounds are dropped."
   , library         = def &= explicit &= name "library" &= name "l" &= help "Only the bounds of the library are dropped."
   , executable      = def &= help "Only the bounds of the executable are dropped."
   , testSuite       = def &= help "Only the bounds of the test suite are dropped."
   , benchmark       = def &= help "Only the bounds of the benchmark are dropped."
   , only            = def &= explicit &= name "only" &= name "O" &= help "Only the bounds of the dependency are modified."
   , ignore          = def &= explicit &= name "ignore" &= name "I" &= help "This dependency is ignored, not modified in any way."
   , outputCabalFile = def &= explicit &= name "outputCabalFile" &= name "o" &= help "Save modified cabal file to file, if empty, the cabal file is modified inplace."
   , cabalFile       = def &= argPos 0 &= typ "CABAL-FILE"
   }

updateArgs :: Args
updateArgs = Update
   { lower           = def &= explicit &= name "lower" &= name "L" &= help "Only the lower bound is updated."
   , upper           = def &= explicit &= name "upper" &= name "U" &= help "Only the upper bound is updated."
   , library         = def &= explicit &= name "library" &= name "l" &= help "Only the bounds of the library are updated."
   , executable      = def &= help "Only the bounds of the executable are updated."
   , testSuite       = def &= help "Only the bounds of the test suite are updated."
   , benchmark       = def &= help "Only the bounds of the benchmark are updated."
   , only            = def &= explicit &= name "only" &= name "O" &= help "Only the bounds of the dependency are modified."
   , ignore          = def &= explicit &= name "ignore" &= name "I" &= help "This dependency is ignored, not modified in any way."
   , outputCabalFile = def &= explicit &= name "outputCabalFile" &= name "o" &= help "Save modified cabal file to file, if empty, the cabal file is modified inplace."
   , cabalFile       = def &= argPos 0 &= typ "CABAL-FILE"
   , setupConfigFile = def &= argPos 1 &= typ "SETUP-CONFIG-FILE"
   }

That's the case for cmdargs 0.10.7. Any ideas? Thanks!

Greetings, Daniel

dan-t commented 10 years ago

Oh, sorry, it's the difference in the help text, which makes perfectly sense. Thanks for this great library!

Greetings, Daniel

ndmitchell commented 10 years ago

Glad you like it - I was actually checking out cabal-bounds just minutes ago :-)

If you have multiple fields that are the same, you can omit them from subsequent constructors, and they will be inferred to be the same as before. e.g. no need to include only in updateArgs.

dan-t commented 10 years ago

On Tue, Feb 25, 2014 at 01:23:46AM -0800, Neil Mitchell wrote:

If you have multiple fields that are the same, you can omit them from subsequent constructors, and they will be inferred to be the same as before. e.g. no need to include only in updateArgs.

Thanks for the hint! :)

Perhaps the only additional thing I would like to have in cmdargs is the ability to give multiple values to an option. I think that's not possible at the moment, right?

So it would be nice to be able to write '--executable=exe1,exe2' instead of '--executable=exe1 --executable=exe2'.

Greetings, Daniel

ndmitchell commented 10 years ago

The only issue with exe1,exe2 is that it wouldn't know if commas make sense as a valid single atom for a String, so you can't always split. One thing you can do is post-process the output of cmdargs, so take all executable fields and do a {executable = concatMap splitOnComma executable}, then you can have comma splitting back.

dan-t commented 10 years ago

On Tue, Feb 25, 2014 at 02:52:28AM -0800, Neil Mitchell wrote:

The only issue with exe1,exe2 is that it wouldn't know if commas make sense as a valid single atom for a String, so you can't always split.

Yes, that's true.

But the splitter could be specified by the user, something like:

executable = def &= splitter "," &= ...

Perhaps better that the splitter isn't specified for every option but for all of them.

Would you consider a patch with such an addition?

One thing you can do is post-process the output of cmdargs, so take all executable fields and do a {executable = concatMap splitOnComma executable}, then you can have comma splitting back.

Yes, perhaps I will tackle it that way.

Greetings, Daniel

ndmitchell commented 10 years ago

I suspect the correct thing to do is to add parser :: Either String a -> Ann, so you can write an arbitrary parser for any type. Once you have that, adding splitter on top of parser is easy. However, writing parser is a bit tricky because the type doesn't force a, so perhaps it needs to be a val -> val type. I haven't looked in to the details. I'd willingly accept a patch for either splitter or parser.

dan-t commented 10 years ago

I think that I might prefer the splitter one, because you could use this information for the help text. So by having a splitter "," you could e.g. display:

-e --executable=ITEM[,...]

Greetings, Daniel

dan-t commented 10 years ago

Hi Neil,

I have a bit of a hard time groking the CmdArgs code base.

I can see that the first step of adding a 'splitter' would be extending the 'Ann' data structure with a new constructor.

The 'Ann's are stored in 'Capture' which is used to initialize 'Prog'. Inside of 'Prog' there's 'Flag', which has the function 'flagValue', which looks like it could be useful for implementing the 'splitter', but the type of the function is 'String -> a -> Either String a', and I don't quite get why the function gets an 'a' and returns one.

Also if would like to modify the help message like described, then I don't quite see where I should apply the modification.

Well all these mappings and remappings make it really hard to follow.

Greetings, Daniel

ndmitchell commented 10 years ago

Sorry for the delay in replying, things got a bit hectic at work. The truth is that the CmdArgs code base can be a little opaque - I plan to give it a good refactoring at some point, but haven't yet had the time, or quite figured out what the result would look like. The Global vs Local split is also quite hard to follow.

The idea behind flagValue is that if your mode is of type A, then flagValue takes the argument the user entered, and the existing A, and returns a Left error message (e.g. failed to parse) or a Right new value of A, e.g. a{myfield = newcontents}.

Applying the help message should be easier, somewhere around https://github.com/ndmitchell/cmdargs/blob/master/System/Console/CmdArgs/Implicit/Local.hs#L202 I would have thought.