kontent-ai / model-generator-net

Kontent.ai .NET model generator.
https://www.nuget.org/packages/Kontent.Ai.ModelGenerator
MIT License
17 stars 18 forks source link

Unrecognized option '--projectid' #45

Closed hades200082 closed 6 years ago

hades200082 commented 6 years ago

--ProjectID and -p both work but --projectid gives the error "Unrecognized option '--projectid'"

petrsvihlik commented 6 years ago

Confirmed, it's a bug.

kashifsoofi commented 6 years ago

A CommandOption can be resolved with 3 names, symbol, short name or long name e.g. "-?|-h|--help", this can be resolved by treating the all small long name as short name so would have to be replaced with -projectid. It can be a work around and this cannot be termed as a bug (only a limitation) as its not something supported by command line parser in use.

hades200082 commented 6 years ago

--ProjectID and --projectid should be interchangeable. Can this not be handled by forcing a lowercase comparison somewhere? (not got the code in front of my right now)

petrsvihlik commented 6 years ago

The parsing's done here.

Ideally, I'd like to preserve both --ProjectID and --projectid. The PascalCase one is necessary for the configuration to work correctly (afaik).

So either we find a way to handle this by lowercasing it at some point or we replace the parser. With System.CommandLine for instance.

hades200082 commented 6 years ago
if (part.StartsWith("--"))
                {
                    LongName = part.Substring(2);
                }

Add .ToLower() on the end of part.Substring(2) then just ensure that everywhere you check it you use only the lowercase variant.

petrsvihlik commented 6 years ago

The code is part of a referenced nuget package...we won't be changing that.

kashifsoofi commented 6 years ago

As you can see CommandLineUtils.CommandOption would only take the last option name, in our case 2nd -- in template would overwrite the -- lower case option in LongName. Both options with all lowercase or PascalCase cannot be used with current referenced package, so in my opinion its not a bug rather limitation of current parser. Another hack can be to have another option with all lowercase.

petrsvihlik commented 6 years ago

It actually is a bug because it worked before we introduced the PascalCase option.

Hmm...interesting idea with the second options...but I'd rather avoid hacks. Let's come up with a solid design and solve it once and for all :)

petrsvihlik commented 6 years ago

fixed by @kashifsoofi in #47