Commit messages are prettified by default when the history is rewritten

[nulltoken]

cf @amibar's findings in #619

• Now: Prevent rewriting of the commit messages and fix the tests so they don't fail because of this.
• Later: Maybe should we find a way to extend RewriteHistoryOptions and let the caller control this behavior?

[nulltoken]

/cc @ben @dahlbyk

[dahlbyk]

I'm cool with adding an option, but by default I'd suggest we go with the breaking change and not prettify without permission.

[ben]

I'm cool with adding an option, but by default I'd suggest we go with the breaking change and not prettify without permission.

This, and what @carlosmn said on the other thread. I'd go further, and say that this shouldn't be an option at all; if the caller wants the message to be prettified, she can prettify it on her own.

[nulltoken]

Ok. Let's start with the breaking change. We'll think about adding the option later, if that is ever required.

[carlosmn]

About the option: this is history rewriting, which means it's just as likely that someone else wrote the message, on another machine at another time. This means that we have zero idea what their comment character is. Even if changing it is not the most common option, prettifying a message from someone else without user confirmation can lose arbitrary parts of a message (e.g. a line where it happened to be that the first char was '#' because they were referencing an issue number, or a markdown heading, or...)

[nulltoken]

@carlosmn You're making a very good point.

[dahlbyk]

if the caller wants the message to be prettified, she can prettify it on her own.

Pretty sure we don't publicly expose prettify, nor do I expect we wish to?

I'm also cool with no option.

[ben]

Pretty sure we don't publicly expose prettify, nor do I expect we wish to?

Whether we do or not, I want to put the caller in the driver's seat here. If they want to rot13-encode the whole thing, turn it upside down, or just rewrap it according to their project's rules, that's up to them.

Now, having a public Commit.PrettifyMessage() might be a nice thing, too, I don't have much of an opinion there. It looks like git_message_prettify strips leading and trailing newlines, and can also kill the comment lines. That might be useful.

[ethomson]

@nulltoken This seems like a bug to me, and / or an additive API change to RewriteHistoryOptions...? I think we can drop the stabilization tag here (but I may not understand this issue fully.)

[nulltoken]

I think we can drop the stabilization tag here (but I may not understand this issue fully.)

Agreed