Clean commit and tag messages #580

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@nulltoken
Member

'git commit' and 'git tag -a' enforce some conventions, like cleaning up excess whitespace and making sure that the last line ends with a '\n'. This fix replicates this behavior.

Fix libgit2/libgit2sharp#117

/cc @peff ready for review, sir!

nulltoken added some commits Mar 3, 2012
@nulltoken nulltoken Fix MSVC compilation warnings 490c7e5
@nulltoken nulltoken commit/tag: ensure the message is cleaned up
'git commit' and 'git tag -a' enforce some conventions, like cleaning up excess whitespace and making sure that the last line ends with a '\n'. This fix replicates this behavior.

Fix libgit2/libgit2sharp#117
bec71a7
@peff
Member
peff commented Mar 6, 2012

Looks reasonable to me. I'm slightly concerned that there's no way to turn off the prettify behavior. 99% of the time, it's what people will want, but libgit2 is a library, so I would expect to be able to use it to do weird things like shoving binary crap into a commit message. Or even just turn off the "ignore comments" behavior. Git, for example, will ignore comments if it generated a comment-laden template for the editor, but will not remove comments from the input to "git commit -F".

I'm not sure of the best way to expose these options to callers. Most callers won't care, and we don't want to burden them with a bunch of extra arguments. Making an options struct might be reasonable. I don't know. We're getting into philosophy of library design, so I'll defer to @tanoku.

@nulltoken
Member

@peff Thanks a lot for having taken the time to review this.

I'll update the PR according to @tanoku's recommendation.

@nulltoken
Member

@tanoku Hey Vicent would you have some spare time to review this, please?

@vmg
Member
vmg commented Mar 10, 2012

@nulltoken: reviewed. But no new feature merges until we're done with the error revamp. We're on it! ❤️

@nulltoken
Member

@tanoku Would you like me to rebase this PR (commit/tag message cleaning) on top of the new-error-handling branch?

@vmg
Member
vmg commented Mar 14, 2012

Please do. That'll make my life so much easier.

@nulltoken nulltoken closed this Mar 14, 2012
@nulltoken
Member

Done :) cf. PR #597

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment