Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: mg: handle C-u in M-! and M-|
To:
tech@openbsd.org
Date:
Tue, 09 Jul 2024 12:23:30 +0200

Download raw body.

Thread
Omar Polo <op@omarpolo.com> wrote:
> florian noticed that this doesn't deal well when invoked on a read-only
> buffer, so check if the buffer is writeable in shellcmdoutput()

there was a bug in the line accounting.  When using plain C-! we didn't
reset mgwin current line number, so multiple C-! would just add to the
line number.  This because bclear() only touches the buffer, not the
window, and the two of them would go out-of-sync.

however, since both mg and Emacs just jump back at the start of the
buffer, add a gotobob() call in the "special" case too.  That will take
care of fixing mgwin' accounting too.

One last touch: save the current position and restore it when adding
text to a buffer instead of leaving the point after the inserted text.
This is what emacs does, so try to be consistent.  There's a small
imperfection yet: when the buffer is empty we consider the current
position to be "at end of buffer" while emacs consider it "beginning of
the buffer".  Not sure I want to go deeper in this rabbit hole just for
this edge case however.


Index: region.c
===================================================================
RCS file: /home/cvs/src/usr.bin/mg/region.c,v
diff -u -p -r1.44 region.c
--- region.c	28 Mar 2023 14:47:28 -0000	1.44
+++ region.c	9 Jul 2024 10:15:23 -0000
@@ -27,14 +27,13 @@
 
 #define TIMEOUT 10000
 
-static char leftover[BUFSIZ];
-
 static	int	getregion(struct region *);
 static	int	iomux(int, char * const, int, struct buffer *);
 static	int	preadin(int, struct buffer *);
 static	void	pwriteout(int, char **, int *);
 static	int	setsize(struct region *, RSIZE);
-static	int	shellcmdoutput(char * const, char * const, int);
+static	int	shellcmdoutput(char * const, char * const, int,
+		    struct buffer *);
 
 /*
  * Kill the region.  Ask "getregion" to figure out the bounds of the region.
@@ -413,13 +412,10 @@ int
 piperegion(int f, int n)
 {
 	struct region region;
+	struct buffer *bp = NULL;
 	int len;
 	char *cmd, cmdbuf[NFILEN], *text;
 
-	/* C-u M-| is not supported yet */
-	if (n > 1)
-		return (ABORT);
-
 	if (curwp->w_markp == NULL) {
 		dobeep();
 		ewprintf("The mark is not set now, so there is no region");
@@ -443,7 +439,14 @@ piperegion(int f, int n)
 
 	region_get_data(&region, text, len);
 
-	return shellcmdoutput(cmd, text, len);
+	if (n > 1) {
+		bp = curbp;
+		undo_boundary_enable(FFRAND, 0);
+		killregion(FFRAND, 1);
+		kdelete();
+	}
+
+	return (shellcmdoutput(cmd, text, len, bp));
 }
 
 /*
@@ -452,33 +455,51 @@ piperegion(int f, int n)
 int
 shellcommand(int f, int n)
 {
+	struct buffer *bp = NULL;
 	char *cmd, cmdbuf[NFILEN];
 
 	if (n > 1)
-		return (ABORT);
+		bp = curbp;
 
 	if ((cmd = eread("Shell command: ", cmdbuf, sizeof(cmdbuf),
 	    EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
 		return (ABORT);
 
-	return shellcmdoutput(cmd, NULL, 0);
+	return (shellcmdoutput(cmd, NULL, 0, bp));
 }
 
 int
-shellcmdoutput(char* const cmd, char* const text, int len)
+shellcmdoutput(char* const cmd, char* const text, int len,
+    struct buffer *bp)
 {
-	struct buffer *bp;
+	struct mgwin *wp;
+	struct line *tlp;
 	char	*argv[] = {NULL, "-c", cmd, NULL};
 	char	*shellp;
-	int	 ret;
+	int	 tbo, ret, special = 0;
 
-	bp = bfind("*Shell Command Output*", TRUE);
-	bp->b_flag |= BFREADONLY;
-	if (bclear(bp) != TRUE) {
-		free(text);
+	if (bp == NULL) {
+		special = 1;
+		bp = bfind("*Shell Command Output*", TRUE);
+		bp->b_flag &= ~BFREADONLY;	/* disable read-only */
+		wp = popbuf(bp, WNONE);
+		if (wp == NULL || bclear(bp) != TRUE) {
+			free(text);
+			return (FALSE);
+		}
+		curbp = bp;
+		curwp = wp;
+	}
+
+	if (bp->b_flag & BFREADONLY) {
+		dobeep();
+		ewprintf("Buffer is read-only");
 		return (FALSE);
 	}
 
+	tlp = curwp->w_dotp;			/* save current position */
+	tbo = curwp->w_doto;
+
 	if ((shellp = getenv("SHELL")) == NULL)
 		shellp = _PATH_BSHELL;
 
@@ -488,14 +509,21 @@ shellcmdoutput(char* const cmd, char* co
 		argv[0] = shellp;
 
 	ret = pipeio(shellp, argv, text, len, bp);
-
 	if (ret == TRUE) {
 		eerase();
-		if (lforw(bp->b_headp) == bp->b_headp)
+		if (special && lforw(bp->b_headp) == bp->b_headp)
 			addline(bp, "(Shell command succeeded with no output)");
 	}
 
 	free(text);
+
+	if (special) {
+		bp->b_flag |= BFREADONLY;	/* restore read-only */
+		gotobob(0, 0);
+	} else {
+		curwp->w_dotp = tlp;		/* return to old position */
+		curwp->w_doto = tbo;
+	}
 	return (ret);
 }
 
@@ -540,7 +568,11 @@ pipeio(const char* const path, char* con
 	default:
 		/* Parent process */
 		close(s[1]);
+
+		undo_boundary_enable(FFRAND, 0);
 		ret = iomux(s[0], text, len, outbp);
+		undo_boundary_enable(FFRAND, 1);
+
 		waitpid(pid, NULL, 0); /* Collect child to prevent zombies */
 
 		return (ret);
@@ -585,13 +617,6 @@ iomux(int fd, char* const text, int len,
 	}
 	close(fd);
 
-	/* In case if last line doesn't have a '\n' add the leftover
-	 * characters to buffer.
-	 */
-	if (leftover[0] != '\0') {
-		addline(outbp, leftover);
-		leftover[0] = '\0';
-	}
 	if (nfds == 0) {
 		dobeep();
 		ewprintf("poll timed out");
@@ -601,7 +626,7 @@ iomux(int fd, char* const text, int len,
 		ewprintf("poll error");
 		return (FALSE);
 	}
-	return (popbuftop(outbp, WNONE));
+	return (TRUE);
 }
 
 /*
@@ -630,42 +655,17 @@ pwriteout(int fd, char **text, int *len)
 }
 
 /*
- * Read some data from socket fd, break on '\n' and add
- * to buffer. If couldn't break on newline hold leftover
- * characters and append in next iteration.
+ * Read some data from socket fd and add to buffer.
  */
 int
 preadin(int fd, struct buffer *bp)
 {
-	int len;
-	char buf[BUFSIZ], *p, *q;
+	char buf[BUFSIZ];
+	ssize_t len;
 
-	if ((len = read(fd, buf, BUFSIZ - 1)) <= 0)
+	if ((len = read(fd, buf, BUFSIZ)) <= 0)
 		return (FALSE);
 
-	buf[len] = '\0';
-	p = q = buf;
-	if (leftover[0] != '\0' && ((q = strchr(p, *bp->b_nlchr)) != NULL)) {
-		*q++ = '\0';
-		if (strlcat(leftover, p, sizeof(leftover)) >=
-		    sizeof(leftover)) {
-			dobeep();
-			ewprintf("line too long");
-			return (FALSE);
-		}
-		addline(bp, leftover);
-		leftover[0] = '\0';
-		p = q;
-	}
-	while ((q = strchr(p, *bp->b_nlchr)) != NULL) {
-		*q++ = '\0';
-		addline(bp, p);
-		p = q;
-	}
-	if (strlcpy(leftover, p, sizeof(leftover)) >= sizeof(leftover)) {
-		dobeep();
-		ewprintf("line too long");
-		return (FALSE);
-	}
+	region_put_data(buf, len);
 	return (TRUE);
 }