Skip to content

Topic/refs fromglob #785

Merged
merged 2 commits into from Jun 26, 2012

5 participants

@nulltoken
libgit2 member

This PR extracts some logic from git_revwalk_(push|hide)_glob() and introduces a new function: git_reference_fromglob().

Note: This includes the #784 fix.

@travisbot

This pull request passes (merged 4620687e into f7292a9).

@nulltoken
libgit2 member

/cc @tclem this may come handy with libgit2/libgit2sharp#184 and allow the following

[Fact]
public void CanEnumerateCommitsUsingGlob()
{
    AssertEnumerationOfCommits(
        repo => new Filter { Since = repo.Refs.FromGlob("heads") },
        new[]
             {
                 "4c062a6", "e90810b", "6dcf9bf", "a4a7dce", "be3563a", "c47800c", "9fd738e", "4a202b3", "41bc8c6", "5001298", "5b5b025", "8496071"
             });
}
@tclem
libgit2 member
tclem commented Jun 22, 2012

Oooo, nice!

@carlosmn
libgit2 member

I seem to always nitpick on style, but wouldn't it make more sense for the function to be called git_reference_foreach_glob? It does what git_reference_foreach does, but adds a glob filter.

On the side of the defaults (defaulting to refs/, adding /* and such), those rules were in the revwalk because those are in git-log so it would make it more intuitive to use in scripting. This function is more like git-for-each-ref, which doesn't have these rules (as it's lower-level, at least on the git side). We could ignore this and claim that it makes it consistent in the library and screw git, specially as those rules should only come into play if the pattern would be the wrong thing to pass to for-each-ref in the first place. Just wanted you to be aware.

@nulltoken
libgit2 member

I seem to always nitpick on style, but wouldn't it make more sense for the function to be called git_reference_foreach_glob? It does what git_reference_foreach does, but adds a glob filter.

I agree. It's far better. Please keep on nitpicking !! :)

We could ignore this and claim that it makes it consistent in the library and screw git, specially as those rules should only come into play if the pattern would be the wrong thing to pass to for-each-ref in the first place. Just wanted you to be aware.

I'm not sure to completely follow you. Are you proposing to drop the normalization and let the caller handle this (eg. passing "refs/heads/*" instead of "heads")? Please feel free to correct me if I've misunderstood your statement.

@travisbot

This pull request passes (merged 1163434 into 798e4d5).

@nulltoken
libgit2 member

@carlos Fixed.

@carlosmn
libgit2 member

Thanks. Looks good.

@vmg vmg merged commit c671339 into libgit2:development Jun 26, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.