Should we allow Configuration to parse a local repository config?

[nulltoken]

@shiftkey in #1031 made an amazing job getting rid of the optional parameters in favor of overloads. While taking a look at the changes in Configuration, I wondered if there was a use case where one would need to access the config of a repository without instantiating it.

Should this need be fulfilled, we'd rather make it happen in v0.22 as it will be quite a nasty breaking change.

/cc @jamill @ethomson

[carlosmn]

Would you need to do a breaking change? Why not have a

Configuration.FromRepository(string path)

named constructor which would make sure ther's a repo at path and open a single config file. You should be able to add that at any point.

[nulltoken]

@carlosmn even if we make this method static, it would still be exposed by an instance ot the type. And this could lead to such unexpected code construct

var repo = new Repository(@"c:\a)
var configFromA = repo.Config;
var configFromB = repo.Config.FromRepository(@"c:\b");

[carlosmn]

Well, don't do that? If you do that you're ignoring the documentation which says that's a named constructor plus your IDE telling you it's a static method and you should not use an instance to refer to it.

[nulltoken]

that's a named constructor

There's no such thing in C#

your IDE telling you it's a static method

It's perfectly valid to call a static method from an instance

If you do that you're ignoring the documentation

I think very few people actually read the documentation. Most of them rely on intellisense and try-outs.

• I'd rather prevent the user from shooting him/herself in the foot by making such code construct impossible
• I'd rather not depend on "hoping" he/she'll make the good choice because he/she previously carefully read the documentation

I may indeed have little faith in humanity...

[Therzok]

I may indeed have little faith in humanity...

seconded.

[bording]

@nulltoken, I think you might be confusing static methods with extension methods here. For your code example above to be valid, FromRepository() would have to be declared as:

public static Configuration FromRepository(this Configuration configuration, string path)

but if you do it this way instead:

public static Configuration FromRepository(string path)

the method would work as @carlosmn proposes with no intellisense confusion.

[nulltoken]

public static Configuration FromRepository(string path)

the method would work as @carlosmn proposes with no intellisense confusion.

@bording You're totally right! Thanks for having corrected me. (crawling back shamefully under my rock)

[nulltoken]

So, I did this thing.... which is now ready for review.

[jamill]

when does this have to be normalized? Could we have a single expected format for the incoming config file location?

[nulltoken]

This probing was added because @ethomson mentioned once that it might be interesting to not being compelled to pass the full path to the config.

As such, it would accept a working directory, a bare repository folder or a full path to a known config file.

If this is no longer required, I can easily remove it.

[jamill]

👍 I think this looks good!

[nulltoken]

Note to self: drop the Repository member if it's not needed

[nulltoken]

Published as NuGet pre-release package LibGit2Sharp.0.22.0-pre20150606092322