Commit: Introduce commentChar and prettifyMessage in CommitOptions

[Therzok]

I'm still on a Mono which can't build so I'll be using travis to run the unit tests. :D

[Therzok]

I could make it so if you set CommentChar, PrettifyMessage is set to true.

[nulltoken]

How about making this a char??

[Therzok]

What would the benefit of having a nullable do here? We're using default(char) which is \0, same for the defaults in the functions like CreateCommit and prettify_message.

[Therzok]

There is nothing gained from this change. We still have to supply a character to prettify_message and it's the same: default(char) aka \0.

[nulltoken]

How about CommentaryChar or CommentaryMarkerChar?

/cc @carlosmn @dahlbyk

[nulltoken]

There is nothing gained from this change. We still have to supply a character to prettify_message and it's the same: default(char) aka \0.

The defaults are supposed to be set in the constructor of the xxxOptions type. I'd rather not expose \0 as an already set property. Beside this, I see \0 as a libgit2 implementation detail and I'm sure it would make much sense to expose this at at the LibGit2Sharp API level.

[Therzok]

This introduces changes in the test data since prettifying was defaulting to true in CreateCommit.

[Therzok]

/cc @nulltoken

[nulltoken]

True to prettify the message by stripping leading and trailing empty lines, trailing whitespace, and collapsing consecutive empty lines, false otherwise.

[nulltoken]

Please use the constructor to set the default values. I'd suggest to go with PrettifyMessage set to true and CommentaryChar to null. Could you please also describe the default behavior as xml doc on the constructor (see MergeOptions for some inspiration 😉)?

[Therzok]

We should default prettify to true? Okay then, I guess that would revert the changes done to the mock object ids I changed.

[Therzok]

Updated.

[nulltoken]

I'd rather see this as a nullable char and pass \0 to libgit2 when it's indeed null.

[nulltoken]

IIRC we should also check that the char is a single byte one.

Two options:

• Either check it at the CommitOptions level (but I feel weird about throwing in a setter)
• Or check it at this level and throw an InvalidOperationException with an explicit message

/cc @ethomson @carlosmn

[Therzok]

Converting from char to sbyte when the char is 2-byte sized will throw an overflow exception. Wouldn't that suffice?

[nulltoken]

I'm not sure an OverflowException would be that useful for the consumer API. The user wouldn't be able to easily understand what he should do to fix this. I think a special casing here with an explicit message may better communicate what's wrong here.

[Therzok]

I think I fixed everything. I'm not explicitly setting CommentaryChar to null since that's redundant. default(Nullable) is null.

[Therzok]

I'm now generating an exception here. Please tell me if the text below should be changed.

[nulltoken]

How about

Only single byte characters are allowed as commentary characters in a message (eg. '#').

[nulltoken]

Very nice work! Thanks! ✨✨✨