bug-grep
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: --only-matching interacts badly with context lines and line head [bu


From: Charles Levert
Subject: Re: --only-matching interacts badly with context lines and line head [bug-grep]
Date: Wed, 23 Feb 2005 07:21:14 -0500
User-agent: Mutt/1.4.1i

* On Wednesday 2005-02-23 at 10:05:05 +0000, Julian Foad wrote:
> Charles Levert wrote:
> >First, it is obvious that --only-matching does not agree
> >with context lines.  I propose the following wholesale
> >fix in main():
> >
> >  if (only_matching)
> >    out_after = out_before = 0;
> 
> Good point, though we probably should error rather than silently ignore 
> part of the user's command.

Maybe, maybe not.  After all, it is possible to interpret
this as "sure, we are accounting for the context lines,
there just aren't any matches to report in them".


> >I propose that a line head, if any is specified, be printed
> >before each match.
> 
> I agree.
> 
> >Third, assuming the previous proposal is retained, I
> >propose that combining --only-matching and --byte-offset
> >print the byte offset of every match itself, as opposed
> >to the byte offset of the beginning of the line on which
> >each match appears.
> 
> I agree.

Cool.


> >I plan to update my patch at
> >
> >  <https://savannah.gnu.org/patch/index.php?func=detailitem&item_id=3644>
> >
> >to implement these proposals, since these affect mostly the
> >same lines that are already heavily modified by the patch.
> >It will always remain possible to ignore this specific
> >version of the patch and use a previous one.
> 
> I appreciate your keenness to provide a patch, but please don't combine it.

I have already done it so that those who want to test
it can, but I absolutely understand the good practice
of feeding things in small independent chunks to CVS,
so that they are easier to roll back if need be.


> I understand your point about it modifying mostly the same lines, but a 
> combined patch will be too hard to review, and would not be suitable for 
> committing to CVS anyway, so it would be of little use.  Instead, let's try 
> to get your existing patch (or parts of it) either committed or rejected, 
> and continue from there, or else write this new bug-fix patch and get it 
> committed (which should be quick, because these bugs are fairly unarguable) 
> and then adapt your patch #3644 to apply on top of these fixes.

As I understand it, the original patch has been accepted on
principle, we're just waiting for the paperwork.  I mailed
it a couple of weeks ago and sent an inquiry yesterday to
make sure that it at least didn't get lost in the mail;
no response yet, but it hasn't even been 24 hours.

Would it be in the realm of possibilities that I could
get CVS access once the paperwork goes through, based on
what you have seen from me so far?



Removing the match_icase stuff in prline() should probably
be done first (in both places) since it is pure deletion
and will make every subsequent patch to this function
smaller.

The "if (only_matching) out_after = out_before = 0;" part
in main(), or the error message alternative, could also
be done independently right away.

My new modification to nlscan() could also be applied by
itself now without adverse effects.

The core of my patch, and separating print_linehead() from
prline(), both touch the same code and could be performed
in either order.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]