Implement porcelain describe

[betesh]

I intend to implement grep as well, following https://stackoverflow.com/questions/15572483/how-to-do-git-grep-e-pattern-with-jgit/16263886#16263886.

It's going to be more work than describe was, so before I move forward, could a maintainer please tell me 1) whether the project would accept such an addition? and 2) whether it should be on a separate branch?

Thank you!

[coveralls]

Coverage increased (+0.009%) to 99.379% when pulling 7c0a62b on betesh:implement-porcelain-describe-and-grep into d718812 on repotag:master.

[bartkamphorst]

Thanks @betesh for the PR! We're happy with the help and will be happy to add Porcelain.describe. A few points still to address though, see the review remarks.

[bartkamphorst]

This line (ending in .call) should be wrapped in a begin rescue end block and rescue the exceptions that DescribeCommand can throw (i.e., MissingObjectException, IncorrectObjectTypeException, RefNotFoundException, IOException).

[betesh]

how should it handle those exceptions? return nil? raise a Ruby exception of some type (what type)?

[bartkamphorst]

Returning nil would be fine.

[betesh]

Done

[bartkamphorst]

Please also add tests for negative cases (wrong input, or no no names found, etc.).

[bartkamphorst]

Regarding grep functionality: the gollum RJGit adapter implements grepping here leveraging RJGit::Porcelain.ls_tree. @dometto Any thoughts on such an approach for RJGit itself? Or would you prefer the approach @betesh referenced in this StackOverflow issue?

[dometto]

Regarding grep: I think I wrote that function for gollum because I thought it would be a) simpler than adding support for JGit's grep to RJGit and b) gollum also wants to match pathname, not just file contents. But support for grep in RJGit would not conflict with that, and I'm all for it! Thanks for chipping in, @betesh!

[betesh]

@dometto It seems like your implementation is much more straightforward? Why not just copy-and-paste it into this repo? (I don't mind writing a few tests, but as far as the code itself, it makes more sense to me for you to push the commit and keep your name on it than for me to paste it and attribute you, especially considering that your LICENSE is empty.)

[dometto]

@betesh I'm fine with using the code from the adapter to implement grep, since JGit doesn't have its own GrepCommand anyway. @bartkamphorst how do you feel about it?

We should add a LICENSE file for sure, but I am not really worried about the attribution, especially since some lines in the snippet would have to be changed/removed anyway (thinking of the Gollum::Git.canonicalize and yield calls for example -- although maybe we could keep the yield as an option?). If you're willing to put in the time to 'port' it to RJGit and write some specs, that would be much appreciated!

[bartkamphorst]

I'm okay with using the adapter code as a basis for the actual implementation. 👍

[betesh]

@bartkamphorst Do you want to merge this and then I'll tackle grep in a separate PR?

[bartkamphorst]

@bartkamphorst Do you want to merge this and then I'll tackle grep in a separate PR?

I have one more suggestion for a minor refactoring, but otherwise it's looking good. Tackling grep in a separate PR is the right way to go. Thanks! 👍