Re: [PATCH v2] config: add --comment option to add a comment

Preface: I am not replying to every part of Junio's email this time
around. However, that does not imply I am ignoring them, I simply do
not currently have access to the source code.

* Junio C Hamano:

 > For readability, you probably would want to have a SP before the
 > given string, i.e.,
 >
 > 	variable = "value" # message comes here

If the user wants a whitespace after the #, they can add it in the comment using quoting, e.g. --comment " message comes here". I don't think it is necessary to enforce the extra SP, because it is not syntactically required. Besides, readability is quite subjective.

 >> The implementation ensures that a # character is always
 >> prepended to the provided comment string, and that the
 >
 > It is unclear what happens when a user gives comment that already
 > has the comment introducer, e.g., --comment="# this is comment".

Unclear? ;-) I wrote "a # character is always prepended to the provided comment string", and that is what happens. The current implementation is meant to be safe, not fancy.

 > No need to repeat a wall of text below the three-dash line if they
 > do not give additional information on top of what is already in the
 > proposed commit log message.

That's GitGitGadget's doing. Unfortunately, it does not want to let me remove my pull request description on the GitHub website. I already tried to fiddle with it, but no success yet; GitHub keeps restoring my latest description text.

 > This topic may want to at least wait for the conclusion of that
 > design discussion, and possibly its implementation.

I don't know enough about the timespans involved to be sure, but that sounds like it could take a long time?

 > I notice that we are sending "comment" to the underlying machinery
 > without doing ANY sanity checking (like, "ah, we got a message
 > without the '#' prefix, so we should add '# ' in front of it before
 > asking the API to add that comment string to the value line").

As I wrote before, the # is prepended unconditionally. LF is a more interesting question. I haven't yet tried actively introducing a linefeed. Does strbuf_addf() have any related filtering logic?

-Ralph