Skip to content

Add a verbose flag to pull-request command #454

Closed
wants to merge 3 commits into from

2 participants

@ivantsepp

git commit has a verbose flag that outputs the diff of the changes you're about to commit. Here, I'm adding the flag that does the same thing: output the diff of the changes that you're going to submit a pull request for.

The diff for git commit -v is not in comments and I think that's done intentionally for text editors to color code the message. So, I did a search for how git knows not to include the diff in the commit message and found this. I've never coded in C but it seems like it searches for diff --git text and ignores everything below it. That's how I implemented it in pull request.

However, that patch also shows the issues with the implementation and how they want to change it to a 'special scissor separator line'. We can change this implementation to use a 'special line' as well but it's not the current git behavior so I didn't do that.

Let me know what you think!

@mislav
GitHub member
mislav commented Dec 25, 2013

I think this is good stuff. But, the new test for the feature is lacking. Specifically, it doesn't verify that the proper diff gets extracted from git. For that to happen, you would need to make actual commits in the test setup and "push" them to appropriate remotes before invoking hub pull-request. However, I admit that this is tricky and something we haven't done in tests before, and I can try to do this myself if you don't figure it out in the meantime.

@ivantsepp

@mislav Thanks for the feedback! I completely agree with you that the tests are lacking in that we don't verify the diff.

I attempted to follow your steps to get in a state where a diff would be displayed. I also attempted to "test" the diff as well (got help here).

I'm not sure if I like what I have so far. My main hiccup is testing the diff because I couldn't find a way to read the contents of PULLREQ_EDITMSG before it was created. But I hope this is the right direction.

@mislav
GitHub member
mislav commented Oct 21, 2014

@ivantsepp Sorry that this hasn't made it in.

Closing this PR as it won't apply anymore since we nuked the Ruby implementation and replaced it with Go. #642

@mislav mislav closed this Oct 21, 2014
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.