From: Dante Catalfamo <dante@lambda.cx>
Subject: Re: mg: grep buffer improvements
To: Omar Polo <op@omarpolo.com>
Cc: tech@openbsd.org
Date: Tue, 04 Jun 2024 23:42:48 -0400

Hey,

compile_goto_error aside, there actually seem to be a couple of
conditions that will cause the existing `next_error` function to crash
mg. If you delete the compile buffer, the window it was using, or
change the buffer in the window it was using, it will create an invalid
state that instantly segfaults. I'll look into that while I'm working
on refining this and maybe submit that as a separate patch.

Dante

On Tue, 2024-06-04 at 09:58 +0200, Omar Polo wrote:
> Hello,
> 
> On 2024/06/02 23:31:17 -0400, Dante Catalfamo <dante@lambda.cx>
> wrote:
> > Hi,
> > 
> > This copies behaviour from Emacs, and makes navigating the `grep`
> > buffer easier and faster.
> 
> Yeah.  This applies to the *compile* buffer too, it's very handy!  I
> use
> it on Emacs all the time.
> 
> > The grep output buffer is marked read-only. Pressing `n` will jump
> > to
> > the next file:line and open it in a split, but will keep focus on
> > the
> > grep buffer. This lets you quickly press `n` so scan through the
> > results. It does the same with `p`, but it goes to the previous
> > result
> > instead of the next.
> 
> unfortunately `p' doesn't work in a *compile* buffer since it doesn't
> search backwards.  The compilers will split several lines of nonsense
> between actual errors but make also shows the commands ran inbetween.
> 
> compile_goto_error() should probably learn to walk the output
> backward
> too, but it doesn't seem a trivial routine.
> 
> some minor style nits below.
> 
> 
> Thanks!
> 
> > +int
> > +grep_next_error(int f, int n)
> > +{
> > +	int ret;
> > +
> > +	if ((ret = next_error(f, n)) != (TRUE)) {
> 
> nit: no need to wrap TRUE in parethesis, and these brances are
> superfluous.  (most of the mg code doesn't use braces when there's
> just
> one statement.)
> 
> > +		return ret;
> > +	}
> > +
> > +	curwp = compile_win;
> > +	curbp = compile_buffer;
> > +
> > +	return (TRUE);
> 
> this wrapping instead is just style, so please keep it.
> 
> > +}
> > +
> > +int
> > +grep_previous_error(int f, int n)
> > +{
> > +	int ret;
> > +
> > +	if ((ret = previous_error(f, n)) != (TRUE)) {
> 
> ditto.
> 
> > +		return ret;
> > +	}
> > +
> > +	curwp = compile_win;
> > +	curbp = compile_buffer;
> > +
> > +	return (TRUE);
> > +}
>