Web lists-archives.com

Re: [PATCH 2/2] t3430: update to test with custom commentChar

Hi Johannes,

On Tue, 10 Jul 2018 at 16:08:57 +0300, Johannes Schindelin wrote:>
On Tue, 10 Jul 2018, Daniel Harding wrote:

On Mon, 09 Jul 2018 at 22:14:58 +0300, Johannes Schindelin wrote:

On Mon, 9 Jul 2018, Daniel Harding wrote:

On Mon, 09 Jul 2018 at 00:02:00 +0300, brian m. carlson wrote:

Should this affect the "# Merge the topic branch" line (and the "# C",
"# E", and "# H" lines in the next test) that appears below this?  It
would seem those would qualify as comments as well.

I intentionally did not change that behavior for two reasons:

a) from a Git perspective, comment characters are only effectual for
if they are the first character in a line


b) there are places where a '#' character from the todo list is actually
parsed and used e.g. [0] and [1].  I have not yet gotten to the point of
grokking what is going on there, so I didn't want to risk breaking
something I
didn't understand.  Perhaps Johannes could shed some light on whether the
cases you mentioned should be changed to use the configured commentChar or


These are related. The first one tries to support

  merge -C cafecafe second-branch third-branch # Octopus 2nd/3rd branch

i.e. use '#' to separate between the commit(s) to merge and the oneline
(the latter for the reader's pleasure, just like the onelines in the `pick
<hash> <oneline>` lines.

The second ensures that there is no valid label `#`.

I have not really thought about the ramifications of changing this to
comment_line_char, but I guess it *could* work if both locations were

Is there interest in such a change?  I'm happy to take a stab at it if there
is, otherwise I'll leave things as they are.

I think it would be a fine change, once we convinced ourselves that it
does not break things (I am a little worried about this because I remember
just how long I had to reflect about the ramifications with regards to the
label: `#` is a valid ref name, after all, and that was the reason why I
had to treat it specially, and I wonder whether allowing arbitrary comment
chars will require us to add more such special handling that is not
necessary if we stick to `#`).

Would it simpler/safer to perhaps put the oneline on its own commented line above? I know it isn't quite consistent with the way onelines are displayed for normal commits, but it might be a worthwhile tradeoff for the sake of the code. As an idea of what I am suggesting, your example above would become perhaps

    # Merge: Octopus 2nd/3rd branch
    merge -C cafecafe second-branch third-branch

or perhaps just

    # Octopus 2nd/3rd branch
    merge -C cafecafe second-branch third-branch


Not that the comment line char feature seems to be all that safe. I could
imagine that setting it to ' ' (i.e. a single space) wreaks havoc with
Git, and we have no safeguard to error out in this obviously broken case.

Technically, I think a single space might actually work with commit messages (at least, I can't off the top of my head think of a case where git would insert a non-comment line starting with a space if it wasn't already present in a commit message). But if someone were actually crazy enough to do that I might suggest a diagnosis of "if it hurts, don't do that" rather than trying to equip git defend against that sort of thing.

Daniel Harding