fix for reading config for branches with dot in name

[thomaswurtz]

When using "git-tfs labels" I got some errors due to the naming of my branches.
I had a branch named after a maintenance version ("maint-1.0.0.0") and since when reading the config a split is done on dot.
I did a quickfix of it. The best options would to not split on dot but that was more work and seemed to require work in libgit2sharp as well.

git-tfs is a great tool by the way 👍 !

[nulltoken]

The best options would to not split on dot but that was more work and seemed to require work in libgit2sharp as well.

Could you please elaborate on this? I'd be happy to help!

[thomaswurtz]

Hmm... actually I think it should be keyParts.Length >= 3 because the additional dots from the branch name will make the Length longer

[thomaswurtz]

@nulltoken, sure this is what I have observed.
The "problem" looks to be that when working with the gitconfig with libgit2sharp one must use dots as separators for the key. For example when reading this configuration:

[tfs-remote "maint-0.9.1"]
url = http://xyz:8080/tfs/DefaultCollection
repository = $/MyDefaultTeamProject/Maint-0.9.1

You get a ConfigurationEntry back where the key is "tfs-remote.maint-0.9.1.url". I was thinking that maybe there could be additional properties to the ConfigurationEntry class instead which could remove the need to build a composite key and parse it when reading. Maybe a category property or something.
I'm not sure whether it is worth the effort to do something about this though :)

[sc68cal]

Could you update your other PR to remove this commit from it, since you've submitted it as a separate PR?

[nulltoken]

@thomaswurtz you could also add the following code (or something similar) to your PR.

public static class ConfigurationEntryExtensions
{
    public static ICollection<string> ToKeyParts<T>(this ConfigurationEntry<T> entry)
    {
        var parts = new List<string>();

        if (entry == null)
        {
            return parts;
        }

        var key = entry.Key;

        var pos1 = key.IndexOf(".", StringComparison.InvariantCulture);
        Debug.Assert(pos1 >= 0);

        parts.Add(key.Substring(0, pos1));

        var pos2 = key.LastIndexOf(".", StringComparison.InvariantCulture);

        if (pos2 < 0 || pos2 == pos1)
        {
            parts.Add(key.Substring(pos1 + 1));
            return parts;
        }

        parts.Add(key.Substring(pos1 + 1, pos2 - pos1 - 1));
        parts.Add(key.Substring(pos2 + 1));

        return parts;
    }
}

[spraints]

I hadn't thought about branches with '.' in the name. Git's configuration shorthand does use '.' to separate parts. In your repo, is the configuration below what git-tfs generated, or did you edit the .git/config file to set it?

[tfs-remote "maint-0.9.1"]
url = http://xyz:8080/tfs/DefaultCollection
repository = $/MyDefaultTeamProject/Maint-0.9.1

If git-tfs generated it, 🆒. Either way, git-tfs should be able to parse it. Your fix looks good to me, but it needs tests (in RemoteConfigConvertLoadTests and maybe in RemoteConfigConverterDumpTests).

[thomaswurtz]

@nulltoken looks good, but where do you think this should be added? I think it looks to belong in libgit2sharp and then I would need to make a separate PR to that repo I assume? or what do you think?

@spraints git-tfs generated the config. Agree that some tests are in place. I added some to the classes you mentioned.

[nulltoken]

@nulltoken looks good, but where do you think this should be added?

@thomaswurtz Well, it's an extension method, it can live everywhere. ;-)

I think it looks to belong in libgit2sharp and then I would need to make a separate PR to that repo I assume? or what do you think?

I spiked it as I thought it might make your life simpler in git-tfs ;-) But you're right, this might be added to LibGit2Sharp as it might benefit to others as well.

Thinking a bit about it, I'm not really satisfied with the name ToKeyParts(), so if you've got a better idea, fell free to express it as part of your PR..

[spraints]

Please inline VerifyMinimalRemote and VerifyCompleteRemote. It makes the tests more self-contained, which makes them easier to look at and verify for correctness. Also, the new tests don't need to check all the keys. Presumably they will still be set right, but checking a couple of them (or maybe just .url, or maybe checking that all the keys start with 'tfs-remote.dotted.branch.name.') shows that the '.' ends up being a '.' in the config keys.