NDesk.Options - spike for better command line argument handling #572

serra · 2015-08-15T11:31:39Z

Work in Progress

This is a spike for #428 to use NDesk.Options to parse command line arguments.
This is the same command line argument parser as used in the Mono project.

First step is to see if we can incorporate this while being completely backwards compatible. This will give us the benefit of a cleaner argument-parsing code base immediately. And it will als allow to add shortcuts immediately and without any pain.

So far seems NDesk.Options can be used while being completely backwards compatible. I think it will take me a couple of hours more to get to a completely backwards compatible implementation.

issues to address

-updateAssemblyInfo takes optional parameter. This is a bit hard to fix while maintaining full backwards compatibility, because for optional values, the notation -updateassemlyinfo filetochange.cs is not allowed; only -updateassemlyinfo:filetochange.cs or -updateassemlyinfo=filetochange.cs. We can only support this if we include it as another special case. (that means excluding it from the option set and handle it as an additional argument)
-execArgs is case sensitive; tested with both execargs and execArgs; probably should create case insensitive option set for backwards compatbility
write tests for untested arguments
update helpwriter
See if we're missing some arguments; there are fields in Arguments.cs that are not tested

Although only -o --option arguments are listed, /o and /option are supported too.

Use convention to derive a SemVer product version from a GitFlow or GitHub based repository.

      --path, --targetpath=VALUE
                             The path to inspect
                               Defaults to the current directory.
  -i, --init                 Start the configuration utility for gitversion
  -h, -?, --help             Show this message and exit
  -l, --log=VALUE            Path to logfile
      --exec=VALUE           Executes target executable making GitVersion 
                               variables available as environmental variables
      --execargs=VALUE       Arguments for the executable specified by --exec
      --proj=VALUE           Build an msbuild file making GitVersion 
                               variables available as msbuild properties
      --projargs=VALUE       Additional arguments to pass to msbuild
  -u, --username=VALUE       Username in case authentication is required
  -p, --password=VALUE       Password in case authentication is required
  -o, --output=VALUE         Determines the output to the console
                               Can be either 'json' or 'buildserver', will 
                               default to 'json'.
      --url=VALUE            Url to remote git repository
  -b, --remotebranch=VALUE   Name of the branch to use on the remote 
                               repository, must be used in combination with --
                               url
      --updateassemblyinfo[=VALUE]
                             * Will recursively search for all 'AssemblyInf-
                               o.cs' files in the git repo and update them
                               To use another filename use --
                               updateassemblyinfo:[another-assemblyinfo-
                               filename.cs]
      --updateassemblyinfoname
                             * Deprecated: use --
                               updateassemblyinfo:[assemblyinfofilename.cs] 
                               instead.
      --dynamicrepolocation=VALUE
                             Override locations dynamic repositories are 
                               clonden to
                               Defaults to %tmp%.
  -c, --commit=VALUE         The commit id to inspect
                               Defaults to the latest available commit on the 
                               specified branch.
  -v, --showvariable=VALUE   Used in conjuntion with /output json, will 
                               output just a particular variable
      --nofetch              
      --showconfig           Outputs the effective GitVersion config
                               Outputs the defaults and custom from GitVersio-
                               n.yaml in yaml format.
      --assemblyversionformat
                             Deprecated: use AssemblyVersioningScheme 
                               configuration value instead.

AS of now, does not compile bco treat warnings as errors or similar project setting.

With validation

…s#428

serra · 2015-08-15T20:05:09Z

But this is not backwards compatible.

Conflicts: src/GitVersionExe/ArgumentParser.cs

serra · 2015-08-16T13:46:35Z

Note that 6e1db37 is NOT backwards compatible.

serra · 2015-08-16T14:46:53Z

@JakeGinnivan I like the state of this PR at 6e1db37.

However, this is a breaking change. It would break any client doing:

gitversion /updateassemlyinfo filetochange.cs

That's currently supported.

What does work is:

gitversion /updateassemlyinfo
gitversion /updateassemlyinfo:filetochange.cs
gitversion /updateassemlyinfo=filetochange.cs
gitversion --updateassemlyinfo:filetochange.cs
gitversion --updateassemlyinfo:filetochange.cs

More specific, -updateAssemblyInfo takes optional parameter. This is a bit hard to fix while maintaining full backwards compatibility, because for optional values, the notation -updateassemlyinfo filetochange.cs is not allowed; only -updateassemlyinfo:filetochange.cs or -updateassemlyinfo=filetochange.cs.

A complete list of accepted arguments can be reviewed here.

We can only support this if we include it as another special case, which would result in some not-so-pretty-yet-easily-identifiable code.

Suggestions? Do we need to be fully backwards compatible?

orjan · 2015-08-16T16:43:48Z

Just a thought, I'm not sure if it's possible or good way, but here's my idea. What about patching the args so they'll work with the new parser? If this is possible it's easy to display that some arguments are deprecated and will be removed in a future version?

On 16 aug 2015, at 16:46, Marijn van der Zee notifications@github.com wrote:

@JakeGinnivan I like the state of this PR at 6e1db37.

However, this is a breaking change. It would break any client doing:

gitversion /updateassemlyinfo filetochange.cs
That's currently supported.

What does work is:

gitversion /updateassemlyinfo
gitversion /updateassemlyinfo:filetochange.cs
gitversion /updateassemlyinfo=filetochange.cs
gitversion --updateassemlyinfo:filetochange.cs
gitversion --updateassemlyinfo:filetochange.cs
More specific, -updateAssemblyInfo takes optional parameter. This is a bit hard to fix while maintaining full backwards compatibility, because for optional values, the notation -updateassemlyinfo filetochange.cs is not allowed; only -updateassemlyinfo:filetochange.cs or -updateassemlyinfo=filetochange.cs.

A complete list of accepted arguments can be reviewed here.

We can only support this if we include it as another special case, which would result in some not-so-pretty-yet-easily-identifiable code.

Suggestions? Do we need to be fully backwards compatible?

—
Reply to this email directly or view it on GitHub.

serra · 2015-08-16T17:47:48Z

@orjan yeah, I did that for other args such as the positional args "init" and "path". I can do that for updateassemlyinfo but it gets kind of messy hence my question @JakeGinnivan if it's worth it or if we can accept a breaking change.

JakeGinnivan · 2015-08-17T00:30:26Z

Before we make this decision, I think we need to look at the command line args again. If we were to go back to the drawing board and come up with new command line arguments, how much better would they be than what we have now. If we can make a drastic improvement to the command line usage by completely breaking it and bumping to v4, then I am leaning towards doing that.

We could always create a GitVersionCompatShim.exe which provides backwards compatibility on a command line level.

JakeGinnivan · 2015-08-17T00:47:41Z

If one of you could come up with what a rethought out command line structure looked like, that would be a great step towards the compat decision

serra · 2015-08-17T06:38:50Z

OK, I'll cook up an overview of proposed command line interface, indicating breaking changes.

SimonCropp · 2015-08-17T10:31:06Z

FWIW I have never liked NDesk.options. it is abandonware and the syntax is ugly and convoluted.

Eg the git repo link here http://www.ndesk.org/Options is dead, even the forks, and the forks of the forks, are abandonware https://github.com/mwpowellhtx/NDesk.Options.Extensions https://github.com/gibbed/NDesk.Options https://github.com/nano-byte/NDesk.Options https://github.com/hach-que/NDesk.Options

I would much prefer something like this https://github.com/gsscoder/commandline

So what other choices apart from NDesk.options were considered?

gep13 · 2015-08-17T10:39:54Z

I know the guys who run Cake are thinking about using the Configuration options that are being cooked up as part of ASP.NET 5 for their tool:

https://www.nuget.org/packages/Microsoft.Framework.Configuration/1.0.0-beta6

https://github.com/aspnet/Configuration

https://gitter.im/cake-build/cake?at=55b919a622f1cbba636fb1a9

Was it considered?

serra · 2015-08-17T15:10:25Z

@SimonCropp afaik NDesk.Options was adopted by the Mono project, see here, history and adoption post from the Mono lead.

I was triggered by this remark requesting for a code dependency instead of nuget/binary dependency. I have not thoroughly considered all command line argument parsers out there, but just went with one I was familiar with. I considered:

[NDesk/Mono].Options (dependency on single codefile, decent commandline interface)
CLAP (impressive, but not really simple IMO)
CommandLineParser (attribute API, would work nicely with current Arguments.cs but would result in strong coupling to the lib)
https://github.com/gsscoder/commandline (just a quick look, similar API as CommandLineParser above)
I did not consider the Cake stuff. Should I?

FWIW I don't intend to do a thorough comparison of all command line argument parsers out there. If anyone has a good reference of such a comparison or a strong preference just let me know and I'll be happy to implement it.

IMO if we use a lib with a proper command line interface (like Mono.Options has) and keep it decoupled well (which the current codebase allows us to do very well BTW, good job there), this can't really hurt us. In fact, this decoupling is wat kept me from picking the CommandLineParser-like API, because the use of attributes IMO give us stronger coupling than we need to the argument parsing library.

Looking forward to your reactions!

dazinator · 2015-08-17T20:08:05Z

I think you could probably implement all of those parsers in a loosely
coupled way - it would just be a matter of defining an 'IParser' contract
with a method that accepts a string array (the args) and returns an
instance of an options class which is a Poco (no attributes etc). You could
have an NDesk implementation of that contract or a CommandLineParser
implementation of that contract etc etc.

I havent used ndesk but CommandLineParser is awesome.

On Mon, 17 Aug 2015 16:10 Marijn van der Zee notifications@github.com
wrote:

@SimonCropp https://github.com/SimonCropp afaik NDesk.Options was
adopted by the Mono project, see here
https://github.com/mono/mono/blob/master/mcs/class/Mono.Options/Mono.Options/Options.cs,
[history(
https://github.com/mono/mono/commits/master/mcs/class/Mono.Options/Mono.Options/Options.cs)
and adoption post from the Mono lead
http://tirania.org/blog/archive/2008/Oct-14.html.

I was triggered by this remark
#428 (comment)
requesting for a code dependency instead of nuget/binary dependency. I have
not thoroughly considered all command line argument parsers out there, but
just went with one I was familiar with. I considered:

[NDesk/Mono].Options (dependency on single codefile, decent
commandline interface)

CLAP http://adrianaisemberg.github.io/CLAP/ (impressive, but not
really simple IMO)

CommandLineParser https://commandline.codeplex.com/ (attribute
API, would work nicely with current Arguments.cs but would result in
strong coupling to the lib)

https://github.com/gsscoder/commandline (just a quick look, similar
API as CommandLineParser above)

I did not consider the Cake stuff. Should I?

FWIW I don't intend to do a thorough comparison of all command line
argument parsers out there
https://www.nuget.org/packages?q=command-line-parser. If anyone has a
good reference of such a comparison or a strong preference just let me know
and I'll be happy to implement it.

IMO if we use a lib with a proper command line interface (like
Mono.Options has) and keep it decoupled well (which the current
codebase allows us to do very well BTW, good job there), this can't really
hurt us. In fact, this decoupling is wat kept me from picking the
CommandLineParser-like API, because the use of attributes IMO give us
stronger coupling than we need to the argument parsing library.

Looking forward to you reactions!

—
Reply to this email directly or view it on GitHub
#572 (comment).

JakeGinnivan · 2015-08-18T02:06:19Z

For me I want to see what the ideal command line args looks like. We should be aligned with git in terms of --longVersion, -l and rethinking the structure so it's easy to work with. Once we know the target command line we want, lets look at which library allows us to achieve that the easiest.

With a rethought cli then we can make the decision if we want to make a breaking change to switch, or if the gains are not worth it?

serra · 2015-08-18T06:03:22Z

@JakeGinnivan I'll write up a proposal today.

serra · 2015-08-19T18:38:45Z

@JakeGinnivan this proposal assumes the same functionality. It is not revolutionary, but more consistent across the cli and allows for extension and renaming.

Main breaking change is that we drop the positional path and init arguments. Breaking changes are marked with an asterix in the description. I can imagine implementing this without breaking changes, which would have the downside of some ugly code and an inconsistent cli wrt the updateassemblyinfo option.

I'd consider it an improvement if option names were better readable, but this can easily be achieved later by specifying aliases for the options and later phasing out bad names along with other breaking changes.

I lack the foresight on what's coming for gitversion. But given the current feature set I'd stick with the current options-only CLI over a verb+options cli (eg git commit -m "intial commit"). If you do foresee the need for this, we should reconsider this proposal.

Use convention to derive a SemVer product version from a GitFlow or GitHub based repository.

      --path, --targetpath=VALUE
                             The path to inspect
                               Defaults to the current directory.
  -i, --init                 Start the configuration utility for gitversion
  -h, -?, --help             Show this message and exit
  -l, --log=VALUE            Path to logfile
      --exec=VALUE           Executes target executable making GitVersion 
                               variables available as environmental variables
      --execargs=VALUE       Arguments for the executable specified by --exec
      --proj=VALUE           Build an msbuild file making GitVersion 
                               variables available as msbuild properties
      --projargs=VALUE       Additional arguments to pass to msbuild
  -u, --username=VALUE       Username in case authentication is required
  -p, --password=VALUE       Password in case authentication is required
  -o, --output=VALUE         Determines the output to the console
                               Can be either 'json' or 'buildserver', will 
                               default to 'json'.
      --url=VALUE            Url to remote git repository
  -b, --remotebranch=VALUE   Name of the branch to use on the remote 
                               repository, must be used in combination with --
                               url
      --updateassemblyinfo[=VALUE]
                             * Will recursively search for all 'AssemblyInf-
                               o.cs' files in the git repo and update them
                               To use another filename use --
                               updateassemblyinfo:[another-assemblyinfo-
                               filename.cs]
      --updateassemblyinfoname
                             * Deprecated: use --
                               updateassemblyinfo:[assemblyinfofilename.cs] 
                               instead.
      --dynamicrepolocation=VALUE
                             Override locations dynamic repositories are 
                               cloned to
                               Defaults to %tmp%.
  -c, --commit=VALUE         The commit id to inspect
                               Defaults to the latest available commit on the 
                               specified branch.
  -v, --showvariable=VALUE   Used in conjunction with /output json, will 
                               output just a particular variable
      --nofetch              
      --showconfig           Outputs the effective GitVersion config
                               Outputs the defaults and custom from GitVersion.yaml in yaml format.
      --assemblyversionformat
                             Deprecated: use AssemblyVersioningScheme 
                               configuration value instead.

serra · 2015-08-19T18:41:50Z

some examples

gitversion
gitversion -i
gitversion /i
gitversion --init
gitversion /?
gitversion --help
gitversion --log=log.txt
gitversion -l log.txt
gitversion --log:log.txt
gitversion -v SemVer
gitversion -v:SemVer

JakeGinnivan · 2015-08-20T08:43:15Z

What about having a more command focused cli, this would align better with git, nuget, npm etc

gitversion init
gitversion help
gitversion version
gitversion debug
gitversion run

If no command is given then run is the default. Then we can show much less when you say gitversion help and then to get more info about running you go gitversion help run?

Thoughts?

JakeGinnivan · 2015-08-20T08:44:55Z

Then run becomes interesting.

I don't like the /output flag, it is confusing and mixed with /log its a mess. We need to think through how to make that better..

serra · 2015-08-20T13:11:42Z

OK, so anything is open?

Agree wrt output and log.

I like the command style cli, but those get messed up IMO when not sticking to verbs (add, commit, log). And they shine for clis to application with much functionality, whereas gitversion is pretty focused.

But it's definitely worth a try. I'll give it some thought and let go of the current cli.

JakeGinnivan · 2015-08-20T13:13:33Z

Small improvements -> Lets try and keep compatible
Large improvements -> Complete rethink/breaking changes are fine

Before trying to get small gains I want to see if we can rethink it to make it a bit more intuitive

pascalberger · 2015-09-29T10:45:55Z

Fyi, there's also PR pending for a possible command line tool in the BCL (dotnet/corefxlab#280).

JakeGinnivan · 2015-09-30T02:54:27Z

I saw that! Looks quite good actually. Need to get back onto this, will try later this week once I sort out Shouldly and TestStack.BDDfy

serra added 16 commits August 14, 2015 21:45

Add NDesk.Options, spike for GitTools#428

3a2ba14

AS of now, does not compile bco treat warnings as errors or similar project setting.

Remove most compilation warnings, for GitTools#428

fb7f54d

Project now compiles, for GitTools#428

7f3bb32

Implement -h, for GitTools#428

5cd297a

Add several parsing examples, for GitTools#428

fc45fda

Add output type validation, for GitTools#428

42a8256

With validation

update assemblyinfo switch option, for GitTools#428

f481c1d

Add nofetch and dynamic repo location options, for GitTools#428

b1970b8

Refactoring of special cases, for GitTools#428

2f35fab

Add case insensitive option set, for GitTools#428

5b0248d

Add target path option with test, for GitTools#428

e1e8498

Add commit id option

0feebe6

Add deprecation warning for assembly version info format, for GitTool…

b189ef9

…s#428

Show config option, for GitTools#428

d188db9

Cleanup and some comments, for GitTools#428

2894d9a

Add init test, for GitTools#428

675f04d

serra changed the title ~~428 n desk spike~~ NDesk.Options - spike for better command line argument handling Aug 15, 2015

serra added 5 commits August 16, 2015 15:10

Update help writer, for GitTools#428

901145d

This is what the updateassemblyinfo should be like, for GitTools#428

99c844a

But this is not backwards compatible.

Update help writer, for GitTools#428

c2c1917

Merge remote-tracking branch 'gittools/master' into 428_NDesk_Spike

18e8f7c

Merge remote-tracking branch 'gittools/master' into 428_NDesk_Spike

6e1db37

Conflicts: src/GitVersionExe/ArgumentParser.cs

vegar mentioned this pull request Aug 18, 2015

Documented commandlineswitch /updateassemblyinfofilename not implemented? #584

Closed

5 tasks

Add deprecated parameter, for GitTools#428

0a3bc68

Last commit for NDesk.Options spike, for GitTools#428

3d796ad

asbjornu mentioned this pull request Jun 16, 2016

GitVersion /showconfig throws exception in Git Bash #905

Closed

serra mentioned this pull request Oct 4, 2016

Command-style cli spike, for #428 #598

Closed

7 tasks

JakeGinnivan closed this Aug 21, 2018

This was referenced May 12, 2020

New CLI #2272

Closed

[Feature] New CLI #2275

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NDesk.Options - spike for better command line argument handling #572

NDesk.Options - spike for better command line argument handling #572

serra commented Aug 15, 2015

serra commented Aug 15, 2015

serra commented Aug 16, 2015

serra commented Aug 16, 2015

orjan commented Aug 16, 2015

serra commented Aug 16, 2015

JakeGinnivan commented Aug 17, 2015

JakeGinnivan commented Aug 17, 2015

serra commented Aug 17, 2015

SimonCropp commented Aug 17, 2015

gep13 commented Aug 17, 2015

serra commented Aug 17, 2015

dazinator commented Aug 17, 2015

JakeGinnivan commented Aug 18, 2015

serra commented Aug 18, 2015

serra commented Aug 19, 2015

serra commented Aug 19, 2015

JakeGinnivan commented Aug 20, 2015

JakeGinnivan commented Aug 20, 2015

serra commented Aug 20, 2015

JakeGinnivan commented Aug 20, 2015

pascalberger commented Sep 29, 2015

JakeGinnivan commented Sep 30, 2015

NDesk.Options - spike for better command line argument handling #572

NDesk.Options - spike for better command line argument handling #572

Conversation

serra commented Aug 15, 2015

issues to address

serra commented Aug 15, 2015

serra commented Aug 16, 2015

serra commented Aug 16, 2015

orjan commented Aug 16, 2015

serra commented Aug 16, 2015

JakeGinnivan commented Aug 17, 2015

JakeGinnivan commented Aug 17, 2015

serra commented Aug 17, 2015

SimonCropp commented Aug 17, 2015

gep13 commented Aug 17, 2015

serra commented Aug 17, 2015

dazinator commented Aug 17, 2015

JakeGinnivan commented Aug 18, 2015

serra commented Aug 18, 2015

serra commented Aug 19, 2015

serra commented Aug 19, 2015

JakeGinnivan commented Aug 20, 2015

JakeGinnivan commented Aug 20, 2015

serra commented Aug 20, 2015

JakeGinnivan commented Aug 20, 2015

pascalberger commented Sep 29, 2015

JakeGinnivan commented Sep 30, 2015