Re: Patch: non-empty matches following empty ones: ChangeLog entry

[Julian Foad]

Sorry, Charles - it was unfriendly of me to criticise your log message without 
praising the work itself, especially as you are the one doing all the work and 
I am doing none.

I do value this change.

My point was that I would appreciate some _extra_ information in the change 
description: specifically, a high-level description of the change's purpose and 
overall effect.  This need only be a few words in many cases.  This will make 
the change log useful to reviewers, people updating the documentation, 
programmers who think the code is doing the wrong thing and who therefore need 
to know why it was changed to do what it does, people writing a list of fixed 
bugs for the release notes, etc.

I believe it is traditional in GNU ChangeLogs _not_ to state the reason for a 
change, but, if that reason is not captured elsewhere (e.g. in a bug tracker), 
there is a serious loss of information.  I hope we can do better than that 
tradition.


Charles Levert wrote:
> Given the single way that print_line_middle()
> is called in the source code, it's implicit that
> this is about --only-matching and --color, and

Yes, but we don't want readers of the change log to have to study how that 
function is/was used in that version of the project in order to understand what 
the change was for.

> * On Friday 2005-08-26 at 11:33:34 +0100, Julian Foad wrote:
> > 2. With "--only-matching" or "--color", don't display or color empty
> >   matches.  (Behaviour change.)
>
> They were not displayed or colorized before,
> so there is no change in behavior there.

Oh, you're right.  I had noticed that with v2.5.1 "-o -n" an empty match was 
printed (at least the line head was printed), and didn't realise that it had 
been fixed already, and your patch to the manuals implied that you were 
changing this behaviour and documenting the result of the change.


> +    * src/grep.c (print_line_middle): In case of an empty match,
> +      make minimal progress and continue instead of aborting process
> +      of the remainder of the line, in case there's still an upcoming
> +      non-empty match.
> +    * tests/foad1.sh: Add two tests for this.

Here, "this" is the main functional fix: that non-empty matches are displayed 
or coloured even after an empty match.  (To over-emphasise a point:  It is not 
specifically testing for any implementation details, and doesn't care whether a 
print_line_middle() function even exists.)

> +    * doc/grep.texi, doc/grep.1: Document this behavior, since

Here, "this" is not that.  Instead, what you documented is that only non-empty 
matches are displayed or coloured.

> +      --only-matching and --color are GNU extensions which are
> +      otherwise unspecified by POSIX or other standards.

(That's good, indicating why it should be necessary to document it.)


So this patch was (a) fixing the bug that non-empty matches were missed after 
an empty match, and (b) documenting the existing intended behaviour w.r.t. 
empty matches in the manuals.  (Yes?)  Excellent.  Knowing that, I can test it 
and be happy with it.  Now, after all this, you see why it would be useful to 
have been told that explicitly... :-)

If you would update the ChangeLog entry (correcting the description of what was 
documented, and adding an overall summary) I'd very much appreciate that.  Or, 
if you'd like me to do so, I can.

And thanks again for continuing to make these fixes to GNU Grep.

- Julian