fclp / fluent-command-line-parser

A simple, strongly typed .NET C# command line parser library using a fluent easy to use interface
Other
530 stars 86 forks source link

Doesn't like parameters which represent URIs #37

Closed ghost closed 9 years ago

ghost commented 9 years ago

Treats / in http:// as the start of a parameter.

For instance:

http://services.internal/backoffce/service/svc

Results in these being seen as AdditionalOptionsFound ... oh and where has Object gone? I can't see it, again could be that it is Saturday and I'm being stupid and need to RTFM to the end.

siywilliams commented 9 years ago

I can't seem to reproduce this, what version of Fclp are you using, and can you provide the setup and args passed in?

I've created some tests to try and find your problem, but they all pass.

If you're using the generic Fclp then the Object property can be found on the Fclp itself: See FluentCommandLineParser.Object. The Object property is not available when using the non-generic Fclp.

Framnk commented 9 years ago

I can reproduce this issue. I am trying to parse a command line that contain an argument with a list of strings and a standard argument as follows:

command.exe -sources http://first/url/path http://second/url/path -v someotherargument

What I've found is that each string in the list of strings is evaluated as a command-line argument, even though it should be ignored as the 'value' of the -sources parameter. So if something in the list of strings happens to match a valid argument that was setup it will be interpreted as such.

siywilliams commented 9 years ago

@Framnk can you give me your setup?

Framnk commented 9 years ago

private class Arguments { public string PackageId { get; set; } public List Sources { get; set; } public string MinVersion { get; set; } public string MaxVersion { get; set; } public bool IncludePreRelease { get; set; } };

var p = new FluentCommandLineParser(); p.Setup(arg => arg.PackageId) .As('p', "PackageId") .WithDescription("Indicates package id or path to packages.config file listing packages to update") .Required();

        p.Setup(arg => arg.MinVersion)
            .As('l', "LowerVersion")
            .WithDescription("Minimum package version to query (only valid when used with specific package id)");

        p.Setup(arg => arg.MaxVersion)
            .As('u', "UpperVersion")
            .WithDescription("Maximum package version to query (only valid when used with specific package id)");

        p.Setup(arg => arg.IncludePreRelease)
            .As('b', "Prerelease")
            .WithDescription("Include pre-release packages when updating");

        p.Setup(arg => arg.Sources)
            .As('s', "sources")
            .WithDescription("Package source(s) to query")
            .Required();

        p.SetupHelp("?", "help")
            .Callback(text => ShowUsage(text));

        var result = p.Parse(args);
        if (result.HasErrors == true)
        {
            p.HelpOption.ShowHelp(p.Options);
            return null;
        }

if I pass the following command-line with this setup it will incorrectly show the URI field as the value of the 'MaxVersion' parameter:

-p Ati.ClientServices.VisionPlugin -sources "https://www.nuget.org/api/v2/"

siywilliams commented 9 years ago

First thing that stands out to me is -sources "https://www.nuget.org/api/v2/" as FCLP uses the GNU style options, where - is a short option, and -- denotes a long option.

Can you try using -p Ati.ClientServices.VisionPlugin --sources "https://www.nuget.org/api/v2/" (note the --) and see if that is causing the problem.

If it still doesn't work at least I've eliminated that from my mind.

siywilliams commented 9 years ago

Also, is public List Sources { get; set; }

supposed to be public List<string> Sources { get; set; }?

Framnk commented 9 years ago

Yes, you're right. It looks like -sources was confusing the parser. It was misleading because it actually WAS filling in my list of sources even though I had specified that parameter incorrectly... I have that marked as a required parameter so I would have thought it would have resulted in a parse error?

As far as the List, it was actually List<> in the code I pasted but that got omitted somehow pasting to Github (not sure how)

siywilliams commented 9 years ago

--sources gets interpreted as short options -s -o -u -r -c -e -s by FCLP. As you've setup sources with a short option -s it matches it and assigns the list of strings to it, which is why is doesn't result in an error.

All very sneaky. I do intend (at somepoint) to add the ability to disable the short option grouping and allow you to specify -sources the way you were, which would have saved you all this trouble!

siywilliams commented 9 years ago

I'm going to close this issue for now as I can't reproduce. @leej0nes if you have any other information that could help me reproduce then re-open this issue and add it. Thanks