From: "Dante Catalfamo" Subject: Re: mg: prevet crash with invalid compile buffer To: "Omar Polo" Cc: tech@openbsd.org Date: Thu, 01 Aug 2024 15:19:23 -0400 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" 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");