Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove constraint on T for ParseArguments with factory #590

Conversation

pergardebrink
Copy link
Contributor

@pergardebrink pergardebrink commented Mar 7, 2020

Fix for #70 and #591

This makes it possible to have mutable types without empty constructor that only does explicit implementation of interfaces, like this (the IsMutable() check would fail previously in this particular case):

public interface IFoo {
    [Option("foobar")]
    string FooBar { get; set; }
}
public interface IBar {
    [Option("barfoo")]
    string BarFoo { get; set; }
}

public class FooBarOptions : IFoo, IBar {
    public FooBarOptions(string nonEmptyConstructor) { }
    IBar.BarFoo { get; set; }
    IFoo.FooBar { get; set; }
}

var result = Parser.Default.ParseArguments<FooBarOptions>(() => new FooBarOptions("foo"), args)

@pergardebrink pergardebrink marked this pull request as ready for review March 7, 2020 23:32
@pergardebrink pergardebrink force-pushed the pergardebrink/removeconstraint branch from 924721e to 28b41f8 Compare March 9, 2020 10:30
@pergardebrink pergardebrink changed the base branch from master to develop March 9, 2020 10:31
@pergardebrink pergardebrink force-pushed the pergardebrink/removeconstraint branch from 28b41f8 to 1a1b88f Compare March 9, 2020 10:32
@moh-hassan
Copy link
Collaborator

Thanks @pergardebrink for your effort.
Nice that you fixed this missing issue.

@moh-hassan moh-hassan added this to the 2.8 milestone Mar 9, 2020
@moh-hassan moh-hassan closed this Mar 9, 2020
@moh-hassan moh-hassan reopened this Mar 9, 2020
@moh-hassan moh-hassan merged commit 3d3d967 into commandlineparser:develop Mar 9, 2020
@moh-hassan
Copy link
Collaborator

Just for correcting typo for the example in your post :

public class FooBarOptions : IFoo, IBar 
{
    public FooBarOptions(string nonEmptyConstructor) { }     
   string  IBar.BarFoo {get;set;}
   string  IFoo.FooBar { get; set; }   
}

@pergardebrink
Copy link
Contributor Author

Thanks @pergardebrink for your effort.
Nice that you fixed this missing issue.

No problem, and thanks for accepting the PR!

I have another use case than my example that relied on having this working, so I was hoping to avoid forking the project just for that reason! ;)

@pergardebrink pergardebrink deleted the pergardebrink/removeconstraint branch March 10, 2020 10:13
moh-hassan pushed a commit that referenced this pull request Mar 11, 2020
* Remove constraint on T for ParseArguments with factory

* Make it possible to use factory for classes without public empty constructor

* Allow types with explicitly defined interface implementations

* Add test for #70

* Make sure explicit interface implementations can be parsed

* Add test to make sure parser can detect explicit interface implementations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants