Found the issuse. If the window is already displaying but not focused, we return true instead of making it the current window, so when next-error checks if we're at the bottom of the buffer, it's looking at whatever we're focused on. 

diff --git a/usr.bin/mg/buffer.c b/usr.bin/mg/buffer.c
index 8c8aab40a..628d485b4 100644
--- a/usr.bin/mg/buffer.c
+++ b/usr.bin/mg/buffer.c
@@ -1091,3 +1091,18 @@ findbuffer(char *fn)
 	bp = bfind(bname, TRUE);
 	return (bp);
 }
+
+/*
+ * Checks if a buffer has a given mode as one of its major or minor
+ * modes.
+ */
+int
+checkbuffermode(struct buffer *bp, char *mode)
+{
+	int i;
+
+	for (i = 0; i <= bp->b_nmodes; ++i)
+		if (!strcmp(bp->b_modes[i]->p_name, mode))
+			return (TRUE);
+	return (FALSE);
+}
diff --git a/usr.bin/mg/def.h b/usr.bin/mg/def.h
index 403635036..56a39c6d3 100644
--- a/usr.bin/mg/def.h
+++ b/usr.bin/mg/def.h
@@ -441,6 +441,7 @@ int		 killbuffer_cmd(int, int);
 int		 savebuffers(int, int);
 int		 listbuffers(int, int);
 int		 addlinef(struct buffer *, char *, ...);
+int		 checkbuffermode(struct buffer *, char *);
 #define	 addline(bp, text)	addlinef(bp, "%s", text)
 int		 anycb(int);
 int		 bclear(struct buffer *);
diff --git a/usr.bin/mg/grep.c b/usr.bin/mg/grep.c
index aa7f9dfd8..f0a237d98 100644
--- a/usr.bin/mg/grep.c
+++ b/usr.bin/mg/grep.c
@@ -29,14 +29,6 @@ void grep_init(void);
 
 static char compile_last_command[NFILEN] = "make ";
 
-/*
- * Hints for next-error
- *
- * XXX - need some kind of callback to find out when those get killed.
- */
-struct mgwin	*compile_win;
-struct buffer	*compile_buffer;
-
 static PF compile_pf[] = {
 	compile_goto_error
 };
@@ -82,7 +74,7 @@ grep(int f, int n)
 	if ((wp = popbuf(bp, WNONE)) == NULL)
 		return (FALSE);
 	curbp = bp;
-	compile_win = curwp = wp;
+	curwp = wp;
 	return (TRUE);
 }
 
@@ -108,7 +100,7 @@ compile(int f, int n)
 	if ((wp = popbuf(bp, WNONE)) == NULL)
 		return (FALSE);
 	curbp = bp;
-	compile_win = curwp = wp;
+	curwp = wp;
 	gotoline(FFARG, 0);
 	return (TRUE);
 }
@@ -166,7 +158,7 @@ gid(int f, int n)
 	if ((wp = popbuf(bp, WNONE)) == NULL)
 		return (FALSE);
 	curbp = bp;
-	compile_win = curwp = wp;
+	curwp = wp;
 	return (TRUE);
 }
 
@@ -240,8 +232,6 @@ compile_mode(const char *name, const char *command)
 	bp->b_modes[1] = name_mode("compile");
 	bp->b_nmodes = 1;
 
-	compile_buffer = bp;
-
 	if (chdir(cwd) == -1) {
 		dobeep();
 		ewprintf("Can't change dir back to %s", cwd);
@@ -261,9 +251,7 @@ compile_goto_error(int f, int n)
 	const char *errstr;
 	struct line	*last;
 
-	compile_win = curwp;
-	compile_buffer = curbp;
-	last = blastlp(compile_buffer);
+	last = blastlp(curbp);
 
  retry:
 	/* last line is compilation result */
@@ -317,16 +305,46 @@ fail:
 	return (FALSE);
 }
 
+static int
+show_compile_buffer(void)
+{
+	struct mgwin	*wp;
+	struct buffer	*bp;
+
+	if (checkbuffermode(curbp, "compile") == TRUE) {
+		return (TRUE);
+	}
+
+	for (bp = bheadp; bp != NULL; bp = bp->b_bufp) {
+		if (checkbuffermode(bp, "compile") == TRUE)
+			goto done;
+	}
+ done:
+	if (bp == NULL)
+		return (FALSE);
+
+	for (wp = wheadp; wp != NULL; wp = wp->w_wndp) {
+		if (wp->w_bufp == bp) {
+			curwp = wp;
+			return (TRUE);
+		}
+	}
+
+	if (splitwind(0, 1) == FALSE ||
+	    showbuffer(bp, curwp, 0) == FALSE)
+		return (FALSE);
+
+	return (TRUE);
+}
+
 int
 next_error(int f, int n)
 {
-	if (compile_win == NULL || compile_buffer == NULL) {
+	if (show_compile_buffer() == FALSE) {
 		dobeep();
 		ewprintf("No compilation active");
 		return (FALSE);
 	}
-	curwp = compile_win;
-	curbp = compile_buffer;
 	if (curwp->w_dotp == blastlp(curbp)) {
 		dobeep();
 		ewprintf("No more hits");



On Thu, Aug 1, 2024, at 2:43 PM, Dante Catalfamo wrote:
> I think this approach makes sense, although there can be two compile 
> buffers, *grep* and *compile*. I tried the patch out, and I seem to get 
> an issue where if I'm not focused on the compile buffer, it will go to 
> the next error once or twice and then say "no more hits" even though 
> the point is only halfway down the buffer.
>
> On Thu, Aug 1, 2024, at 7:09 AM, Omar Polo wrote:
>> On 2024/07/31 13:06:50 -0400, "Dante Catalfamo" <dante@lambda.cx> wrote:
>>> Realized I could open the compile buffer in a new window if it's not associated with one currently.
>>
>> counter-proposal: if, instead of trying to maintain the global pointers
>> that can get invalidated by kill-buffer, in next_error we just search
>> for a buffer in the compile-mode and use it?  mg has only one compile
>> buffer anyway.
>>
>> I don't use often M-x compile or grep, but with some light testing this
>> works for me.  (actually i should resurrect your other diff to jump
>> between hits in the compile buffer and my old diff for M-x grep.)
>>
>> what do you think?
>>
>>
>> diff /home/op/w/mg
>> commit - 80617155f9fa32a62f9dbcbea7b90ebbdb39ec14
>> path + /home/op/w/mg
>> blob - aa7f9dfd87d387253c9d8f8b20b0f47f5589b6fe
>> file + grep.c
>> --- grep.c
>> +++ grep.c
>> @@ -29,14 +29,6 @@ void grep_init(void);
>> 
>>  static char compile_last_command[NFILEN] = "make ";
>> 
>> -/*
>> - * Hints for next-error
>> - *
>> - * XXX - need some kind of callback to find out when those get killed.
>> - */
>> -struct mgwin	*compile_win;
>> -struct buffer	*compile_buffer;
>> -
>>  static PF compile_pf[] = {
>>  	compile_goto_error
>>  };
>> @@ -82,7 +74,7 @@ grep(int f, int n)
>>  	if ((wp = popbuf(bp, WNONE)) == NULL)
>>  		return (FALSE);
>>  	curbp = bp;
>> -	compile_win = curwp = wp;
>> +	curwp = wp;
>>  	return (TRUE);
>>  }
>> 
>> @@ -108,7 +100,7 @@ compile(int f, int n)
>>  	if ((wp = popbuf(bp, WNONE)) == NULL)
>>  		return (FALSE);
>>  	curbp = bp;
>> -	compile_win = curwp = wp;
>> +	curwp = wp;
>>  	gotoline(FFARG, 0);
>>  	return (TRUE);
>>  }
>> @@ -166,7 +158,7 @@ gid(int f, int n)
>>  	if ((wp = popbuf(bp, WNONE)) == NULL)
>>  		return (FALSE);
>>  	curbp = bp;
>> -	compile_win = curwp = wp;
>> +	curwp = wp;
>>  	return (TRUE);
>>  }
>> 
>> @@ -240,8 +232,6 @@ compile_mode(const char *name, const char *command)
>>  	bp->b_modes[1] = name_mode("compile");
>>  	bp->b_nmodes = 1;
>> 
>> -	compile_buffer = bp;
>> -
>>  	if (chdir(cwd) == -1) {
>>  		dobeep();
>>  		ewprintf("Can't change dir back to %s", cwd);
>> @@ -261,9 +251,7 @@ compile_goto_error(int f, int n)
>>  	const char *errstr;
>>  	struct line	*last;
>> 
>> -	compile_win = curwp;
>> -	compile_buffer = curbp;
>> -	last = blastlp(compile_buffer);
>> +	last = blastlp(curbp);
>> 
>>   retry:
>>  	/* last line is compilation result */
>> @@ -317,16 +305,42 @@ fail:
>>  	return (FALSE);
>>  }
>> 
>> +static int
>> +show_compile_buffer(void)
>> +{
>> +	struct mgwin	*wp;
>> +	struct buffer	*bp;
>> +	int		 i;
>> +
>> +	for (bp = bheadp; bp != NULL; bp = bp->b_bufp) {
>> +		for (i = 0; i <= bp->b_nmodes; ++i)
>> +			if (!strcmp(bp->b_modes[i]->p_name, "compile"))
>> +				goto done;
>> +	}
>> + done:
>> +	if (bp == NULL)
>> +		return (FALSE);
>> +
>> +	for (wp = wheadp; wp != NULL; wp = wp->w_wndp) {
>> +		if (wp->w_bufp == bp)
>> +			return (TRUE);
>> +	}
>> +
>> +	if (splitwind(0, 1) == FALSE ||
>> +	    showbuffer(bp, curwp, 0) == FALSE)
>> +		return (FALSE);
>> +
>> +	return (TRUE);
>> +}
>> +
>>  int
>>  next_error(int f, int n)
>>  {
>> -	if (compile_win == NULL || compile_buffer == NULL) {
>> +	if (show_compile_buffer() == FALSE) {
>>  		dobeep();
>>  		ewprintf("No compilation active");
>>  		return (FALSE);
>>  	}
>> -	curwp = compile_win;
>> -	curbp = compile_buffer;
>>  	if (curwp->w_dotp == blastlp(curbp)) {
>>  		dobeep();
>>  		ewprintf("No more hits");