Enable MutuallyExclusiveSet behaviour by default

[serbrech]

Hi, First of all, thanks for the library, I really appreciate the effort and the project in general. :)

But I just spent 1h trying to get MutuallyExclusiveSet feature working. I wrote tests, digged read your test code, and I could not understand why my sets were not working. I finally looked at the source code, and that's where I found that there was a ParserSettings class and the feature flag defaults to false.

The documentation does not specify that you must ENABLE the feature. I just added sets, and they were not working. How are users expected to figure this out?

If this is a the expected behaviour, then I suggest that you at least :

1. document it on the feature's page
2. document it on the OptionAttribute MutuallyExclusive xml docs
3. detect that sets are used but the feature is a disabled and warn the developer

But really... why?

[serbrech]

The doc is wrong as well. MutuallyExclusiveSet option works in exactly the opposite way as documented in the wiki :

class TestOptions
{
    [Option(MutuallyExclusiveSet = "web")]
    public string DocumentRoot { get; set; }

    [Option(MutuallyExclusiveSet = "web")]
    public bool EnableJavaScript { get; set; }

    [Option(MutuallyExclusiveSet = "ftp")]
    public string FtpDirectory { get; set; }

    [Option(MutuallyExclusiveSet = "ftp")]
    public bool AnonymousLogin { get; set; }
}

[Test]
public void TestDocPermitted()
{
    //as per documentation : 
    //;; permitted
    //$ app --enablejavascript --documentroot ~/var/local/website
    var parser = new Parser(settings =>
    {
        settings.MutuallyExclusive = true;
    });
    var options = new TestOptions();
    var parseResult = _parser.ParseArguments(new[] { "--enablejavascript", "--documentroot", "~/var/local/website" }, options);
    parseResult.Should().BeTrue(); //FAILS
}

[Test]
public void TestDocDenied()
{
    //as per documentation : 
    //;; permitted
    //$ app --enablejavascript --documentroot ~/var/local/website
    var parser = new Parser(settings =>
    {
        settings.MutuallyExclusive = true;
    });
    var options = new TestOptions();
    var parseResult = parser.ParseArguments(new[] { "--anonymouslogin", "--enablejavascript"}, options);
    parseResult.Should().BeFalse(); //FAILS
}

[gsscoder]

Hi, sorry for delay (motivated in the last issue tagged announcement) a tweet from another developer brought me here.

I want to understand better. Excluding that documentation may contain inaccuracy, are behaviour and semantic of the library correct?

Are you suggesting to change something or to fix something?

Bye

[serbrech]

Hi,

Well, It's been a while now, but thanks to get back to me. Why do we need to set a flag to enable the feature? Why isn't it on by default?

[gsscoder]

Hello, as I said was outside my country and had personal tasks to accomplish that had priority to everything. Sorry!

So the only thing you want is that this feature is enabled by default and fix documentation? There's no particular reason for that default, except that in my mind this is a feature, so you've to activate it (just a line of code in every .NET laguage, right?).

If this thing will be changed and posted to NuGet, people that use 2.0 instead 1.9 will be forced to change their source; but is also true that this is still a beta and things can be changed.

As Yoda said: meditate on this I will (https://goo.gl/8LBqFW).

I swear that this time you will get a reply in less than very few days.

[serbrech]

My thinking :

The feature does not intersect any other feature : Turning it ON does not change the behaviour of existing code that is not using this feature explicitely. so there is no reason to have it OFF by default. Your library supports it, and if we use it, it works : no surprise.

It is in theory a breaking change to set it on by default, as you can potentially change the behaviour of people relying on it. However, the only case where this would happen, is if they used the MutuallyExclusiveSet and did not turn the feature on explicitely, which sounds more like a bug... Anyone who had it turned on or off explicitely in the settings will see the exact same behaviour.

What I would do is mark the Settings property as Obsolete giving a warning if they update to the latest package, and remove it in the next major (if you come to release one).

and by the way, I'm happy to help of course :)

[gsscoder]

Hi my friend, your help is really appreciated. :) There's hight probability that this will fixed in that way, but as first thing I'll update the wiki (and than re-update after changes; it's a question of consistency).

I'm quite busy in these days, but this mission will be accomplished!

Thanks, again.

[gsscoder]

Hi, I've fixed wiki, please check https://github.com/gsscoder/commandline/wiki/Mutually-Exclusive-Options. I think now it's more clear; it just lacked to say to set the option (it was my fault, I've taken for granted).

But please remember that except chapter Latest Version, the wiki is for latest stables, as stated in the header.

Bye :)

[serbrech]

It's much clearer now 👍

[gsscoder]

Hi, finally after some time (sorry!) I put myselft on work.

I've discovered that beta version already lacks the need to set MutuallyExclusive in ParserSettings (in which the property was removed). Sorry for confusion but I too much time far from the project.

Please confirm it by yourselft and let me know if I'm wrong or not.

Thanks for your patience!