zzarchive-Projekt icon indicating copy to clipboard operation
zzarchive-Projekt copied to clipboard

Address #27 from fsprojects/Projekt

Open baronfel opened this issue 9 years ago • 10 comments

I took a pass at unifying the command line args according to issue #27 . I think CommandLine.Parser will work better for this case, due to the command -> verb -> options pattern this utility has.

I added a basic test that verifies that the new implementation is equivalent to the old implementation, but what I'd really like to do is write an FsCheck test that uses the Operation type to generate a command line string via the old parser, and then parse that string using the new parser and compare the resulting Operations.

baronfel avatar Jul 31 '15 15:07 baronfel

@baronfel: thanks a lot for taking a look at this. How do you think your approach affects the ease of automatically generating the docs from the usage text like Paket does? At the moment I copied and pasted manually into https://github.com/fsprojects/Projekt/blob/master/docs/content/index.md, but I'd like to avoid that in the future.

rneatherway avatar Aug 01 '15 23:08 rneatherway

This library will also do the automatic command line usage generation. The 'description' attribute parameters on the verbs and options are what is used automatically. I can post a screenshot here later after a few pints ;) I think I like this library a bit more than the current rev of UnionArgsParser for verbs.

baronfel avatar Aug 02 '15 00:08 baronfel

image

So this is what it looks like out of the box. But using the included API I can make this look however you like. Any pointers/requests? I'm guessing to keep it like the previous version?

baronfel avatar Aug 03 '15 01:08 baronfel

Looks ok to me however I'd like the code formatting to follow common formatting guidelines a bit closer: https://github.com/dungpa/fantomas/blob/master/docs/FormattingConventions.md, especially around records.

Some lines are also a bit too long. Try keeping them below 120 if possible.

kjnilsson avatar Aug 03 '15 10:08 kjnilsson

Is this more in line? I was able to condense some of the long lines as well as correct the newline/braces situation with the records. I also forgot to annotate the positional parameters with MetaNames, so now the positional parameters have help entries.

baronfel avatar Aug 03 '15 14:08 baronfel

Ok, this commit should address the pieces that you brought up. Thanks for the detail there.

baronfel avatar Aug 03 '15 14:08 baronfel

I really like the look of this! Thanks a lot for the PR. Could you tidy the help output up a bit more though please?

  1. No blank line between options.

  2. Don't output --help and --version in the help for every verb.

  3. Preserve the help text for the options from the old version. For example for direction under movefile, we should say that the choices are (up|down) as before. Same goes for most of the options, just to make it clear what the acceptable range is.

    $ bin/Projekt/Projekt.exe help movefile
    Projekt 0.0.2.0
    Copyright (C) 2015 fsprojects
    
    --direction              Required. 
    
    --repeat                 (Default: 1) 
    
    --help                   Display this help screen.
    
    --version                Display version information.
    
    project path (pos. 0)    Required. 
    
    file path (pos. 1)       Required. 
    
    
  4. Rather than listing the positional arguments like above, is it possible to use the more standard approach of a header of the form: Project.exe movefile <project path> <file path>?

rneatherway avatar Aug 03 '15 16:08 rneatherway

@rneatherway thanks for the feedback on the help text. What we've got there is completely out of the box, and I can customize it to a great extent. I'll spend some time making the text in line with your points.

baronfel avatar Aug 03 '15 16:08 baronfel

There's a bit of a roadblock with this PR, because the version of CommandLine.Parser that supports verbs doesn't allow for the same level of customization as previous versions.

It used to be that you could hijack and overwrite the entire help generation, but now you can only use attributes and examples to guide the automatic generation of help usage, as briefly described in (this issue)[https://github.com/gsscoder/commandline/issues/116].

So I can't really address your concerns right now, @rneatherway. :( Maybe we put this one on the back burned for a bit until I or original dev can extend help generation.

baronfel avatar Aug 05 '15 21:08 baronfel

@baronfel that's a real shame, thanks a lot for taking the time to have a crack at this in any case. Hopefully it will be implemented soon, the project does seem to be actively developed right now and it's a fairly important thing.

rneatherway avatar Aug 06 '15 17:08 rneatherway