Re: [PATCH] dired mg patch
It fixes the issue that if a filename has a space at the start of it, the point will stay in the first character column and not jump to the first non ' ' character in the filename. Yup, this looks good to me. However, it does seem to expose a bug in dired, that if a filename has a space at the start of it, mg doesn't find it if you try and open it. mg gives a message of (New File) and creates and empty buffer. But in the mean time, I think this change does make this diff more correct. Indeed. This looks like a flaw in d_makename, which seems to implement its own warpdot() :-) This should be changed to use our new function (and we could make use of NAME_FIELD). That fix, along with the one for dealing with shell metacharacters, should probably be a separate diff though.
Re: [PATCH] dired mg patch
However, it does seem to expose a bug in dired, that if a filename
has a space at the start of it, mg doesn't find it if you try and
open it. mg gives a message of (New File) and creates and empty
buffer. But in the mean time, I think this change does make this
diff more correct.
Indeed. This looks like a flaw in d_makename, which seems to implement
its own warpdot() :-) This should be changed to use our new function
(and we could make use of NAME_FIELD). That fix, along with the one
for dealing with shell metacharacters, should probably be a separate
diff though.
And here's that diff. Makename() uses warpdot(). I also refined warpdot
to use NAME_FIELD and return (FALSE) if the field cannot be located (this
is the change I anticipated earlier). This return value should make a
couple conditions easier to understand.
Then there's ls. Trying to escape shell metacharacters is one of the
uglier things a program can do, if the alternative of passing the string
directly to a program is available. Most of this code (fork exec) is
already in dired.c in shell_command(). I took this code and put it
into a separate function, d_exec(), and added some vararg magic. Using
this function instead of popen() eliminates the issues with metachars
while making dired_() quite a bit simpler.
One bit I'm not particularly fond of is the first parameter of d_exec,
which is used to prefix the lines with spaces (needed so the action chars
like '!' fit on the line in front of ls output). On the principle of YAGNI,
however, I did not implement a bigger hammer that would've let the caller
read process lines one at a time; this would've involved more functions
and state.
I also added a few relevant comments.
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.49
diff -u -p -r1.49 dired.c
--- dired.c 29 Aug 2011 11:02:06 - 1.49
+++ dired.c 29 Aug 2011 13:54:10 -
@@ -21,6 +21,7 @@
#include fcntl.h
#include errno.h
#include libgen.h
+#include stdarg.h
voiddired_init(void);
static int dired(int, int);
@@ -33,6 +34,7 @@ static int d_expunge(int, int);
static int d_copy(int, int);
static int d_del(int, int);
static int d_rename(int, int);
+static int d_exec(int, struct buffer *, const char *, const char *, ...);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
@@ -483,14 +485,10 @@ reaper(int signo __attribute__((unused))
int
d_shell_command(int f, int n)
{
- char command[512], fname[MAXPATHLEN], buf[BUFSIZ], *bufp, *cp;
- int infd, fds[2];
- pid_tpid;
- struct sigaction olda, newa;
+ char command[512], fname[MAXPATHLEN], *bufp;
struct buffer *bp;
struct mgwin*wp;
- FILE*fin;
- char sname[NFILEN];
+ char sname[NFILEN];
bp = bfind(*Shell Command Output*, TRUE);
if (bclear(bp) != TRUE)
@@ -506,11 +504,61 @@ d_shell_command(int f, int n)
bufp = eread(! on %s: , command, sizeof(command), EFNEW, sname);
if (bufp == NULL)
return (ABORT);
- infd = open(fname, O_RDONLY);
- if (infd == -1) {
+
+ if (d_exec(0, bp, fname, sh, -c, command, NULL) != TRUE)
+ return (ABORT);
+
+ if ((wp = popbuf(bp, WNONE)) == NULL)
+ return (ABORT); /* XXX - free the buffer?? */
+ curwp = wp;
+ curbp = wp-w_bufp;
+ return (TRUE);
+}
+
+/*
+ * Pipe input file to cmd and insert the command's output in the
+ * given buffer. Each line will be prefixed with the given
+ * number of spaces.
+ */
+static int
+d_exec(int space, struct buffer *bp, const char *input, const char *cmd, ...)
+{
+ char buf[BUFSIZ];
+ va_list ap;
+ struct sigaction olda, newa;
+ char**argv, *cp;
+ FILE*fin;
+ pid_tpid;
+ int infd, fds[2];
+ int n;
+
+ /* Find the number of arguments. */
+ va_start(ap, cmd);
+ for (n = 2; va_arg(ap, char *) != NULL; n++)
+ ;
+ va_end(ap);
+
+ /* Allocate and build the argv. */
+ if ((argv = calloc(n, sizeof(*argv))) == NULL) {
+ ewprintf(Can't allocate argv : %s, strerror(errno));
+ return (FALSE);
+ }
+
+ n = 1;
+ argv[0] = (char *)cmd;
+ va_start(ap, cmd);
+ while ((argv[n] = va_arg(ap, char *)) != NULL)
+ n++;
+ va_end(ap);
+
+ if (input == NULL)
+ input = /dev/null;
+
+ if ((infd = open(input, O_RDONLY)) == -1) {
ewprintf(Can't open input file : %s, strerror(errno));
return (FALSE);
}
+
if (pipe(fds) == -1) {
ewprintf(Can't create pipe : %s,
Re: [PATCH} dired mg patch
This fixes Nima's PR and mark's issue with
filenames that start with a space.
Index: src/usr.bin/mg/dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- src/usr.bin/mg/dired.c 23 Jan 2011 00:45:03 - 1.48
+++ src/usr.bin/mg/dired.c 29 Aug 2011 15:17:15 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -165,8 +193,12 @@ dired_init(void)
funmap_add(d_findfile, dired-find-file);
funmap_add(d_ffotherwindow, dired-find-file-other-window);
funmap_add(d_del, dired-flag-file-deleted);
+ funmap_add(d_forwline, dired-next-line);
funmap_add(d_otherwindow, dired-other-window);
+ funmap_add(d_backline, dired-previous-line);
funmap_add(d_rename, dired-rename-file);
+ funmap_add(d_backpage, dired-scroll-down);
+ funmap_add(d_forwpage, dired-scroll-up);
funmap_add(d_undel, dired-unflag);
maps_add((KEYMAP *)diredmap, dired);
dobindkey(fundamental_map, dired, ^Xd);
@@ -563,13 +595,13 @@ d_create_directory(int f, int n)
return (showbuffer(bp, curwp, WFFULL | WFMODE));
}
-#define NAME_FIELD 8
static int
d_makename(struct line *lp, char *fn, size_t len)
{
- int i;
+ int i, col;
char*p, *ep;
+ col = 0;
if (strlcpy(fn, curbp-b_fname, len) = len)
return (FALSE);
@@ -577,21 +609,98 @@ d_makename(struct line *lp, char *fn, si
return (ABORT);
ep = lp-l_text + llength(lp);
p++; /* skip action letter, if any */
- for (i = 0; i NAME_FIELD; i++) {
- while (p ep isspace(*p))
- p++;
- while (p
Re: [PATCH] dired mg patch
This diff works fine. Not using metacharacters and passing it as argv is the proper fix. Escaping shell metacharacters isn't the proper way. Warpdot() is also a little bit safer now. It addresses some of the issues I was a bit worried about. This should go in !
Re: [PATCH] dired mg patch
And here's that diff. [..]
It never hurts to re-read a diff before going to bed. Indeed, I
forgot to free argv (execlp uses alloca). The number of things
to clean up after one thing fails is almost getting out of hand,
and at least the fork() case wasn't handled properly. In this new
revision of d_exec, argv and all the fds are checked and freed
(only) at the end of the function, if necessary. That's where we
jump to, should anything fail.
The rest of the diff is as before.
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.49
diff -u -p -r1.49 dired.c
--- dired.c 29 Aug 2011 11:02:06 - 1.49
+++ dired.c 29 Aug 2011 23:27:09 -
@@ -21,6 +21,7 @@
#include fcntl.h
#include errno.h
#include libgen.h
+#include stdarg.h
voiddired_init(void);
static int dired(int, int);
@@ -33,6 +34,7 @@ static int d_expunge(int, int);
static int d_copy(int, int);
static int d_del(int, int);
static int d_rename(int, int);
+static int d_exec(int, struct buffer *, const char *, const char *, ...);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
@@ -483,14 +485,10 @@ reaper(int signo __attribute__((unused))
int
d_shell_command(int f, int n)
{
- char command[512], fname[MAXPATHLEN], buf[BUFSIZ], *bufp, *cp;
- int infd, fds[2];
- pid_tpid;
- struct sigaction olda, newa;
+ char command[512], fname[MAXPATHLEN], *bufp;
struct buffer *bp;
struct mgwin*wp;
- FILE*fin;
- char sname[NFILEN];
+ char sname[NFILEN];
bp = bfind(*Shell Command Output*, TRUE);
if (bclear(bp) != TRUE)
@@ -506,68 +504,124 @@ d_shell_command(int f, int n)
bufp = eread(! on %s: , command, sizeof(command), EFNEW, sname);
if (bufp == NULL)
return (ABORT);
- infd = open(fname, O_RDONLY);
- if (infd == -1) {
+
+ if (d_exec(0, bp, fname, sh, -c, command, NULL) != TRUE)
+ return (ABORT);
+
+ if ((wp = popbuf(bp, WNONE)) == NULL)
+ return (ABORT); /* XXX - free the buffer?? */
+ curwp = wp;
+ curbp = wp-w_bufp;
+ return (TRUE);
+}
+
+/*
+ * Pipe input file to cmd and insert the command's output in the
+ * given buffer. Each line will be prefixed with the given
+ * number of spaces.
+ */
+static int
+d_exec(int space, struct buffer *bp, const char *input, const char *cmd, ...)
+{
+ char buf[BUFSIZ];
+ va_list ap;
+ struct sigaction olda, newa;
+ char**argv = NULL, *cp;
+ FILE*fin;
+ int fds[2] = { -1, -1 };
+ int infd = -1;
+ int ret = (ABORT), n;
+ pid_tpid;
+
+ if (sigaction(SIGCHLD, NULL, olda) == -1)
+ return (ABORT);
+
+ /* Find the number of arguments. */
+ va_start(ap, cmd);
+ for (n = 2; va_arg(ap, char *) != NULL; n++)
+ ;
+ va_end(ap);
+
+ /* Allocate and build the argv. */
+ if ((argv = calloc(n, sizeof(*argv))) == NULL) {
+ ewprintf(Can't allocate argv : %s, strerror(errno));
+ goto out;
+ }
+
+ n = 1;
+ argv[0] = (char *)cmd;
+ va_start(ap, cmd);
+ while ((argv[n] = va_arg(ap, char *)) != NULL)
+ n++;
+ va_end(ap);
+
+ if (input == NULL)
+ input = /dev/null;
+
+ if ((infd = open(input, O_RDONLY)) == -1) {
ewprintf(Can't open input file : %s, strerror(errno));
- return (FALSE);
+ goto out;
}
+
if (pipe(fds) == -1) {
ewprintf(Can't create pipe : %s, strerror(errno));
- close(infd);
- return (FALSE);
+ goto out;
}
newa.sa_handler = reaper;
newa.sa_flags = 0;
- if (sigaction(SIGCHLD, newa, olda) == -1) {
- close(infd);
- close(fds[0]);
- close(fds[1]);
- return (ABORT);
+ if (sigaction(SIGCHLD, newa, NULL) == -1)
+ goto out;
+
+ if ((pid = fork()) == -1) {
+ ewprintf(Can't fork);
+ goto out;
}
- pid = fork();
+
switch (pid) {
- case -1:
- ewprintf(Can't fork);
- return (ABORT);
- case 0:
+ case 0: /* Child */
close(fds[0]);
dup2(infd, STDIN_FILENO);
dup2(fds[1], STDOUT_FILENO);
dup2(fds[1], STDERR_FILENO);
- execl(/bin/sh, sh, -c, bufp, (char *)NULL);
+ execvp(argv[0], argv);
exit(1);
break;
- default:
+ default: /* Parent */
Re: [PATCH] dired mg patch
The latest diff has no issues so far.
I just added the error message in case
execvp() fails.
Index: src/usr.bin/mg/dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.49
diff -u -p -r1.49 dired.c
--- src/usr.bin/mg/dired.c 29 Aug 2011 11:02:06 - 1.49
+++ src/usr.bin/mg/dired.c 30 Aug 2011 03:39:11 -
@@ -21,6 +21,7 @@
#include fcntl.h
#include errno.h
#include libgen.h
+#include stdarg.h
voiddired_init(void);
static int dired(int, int);
@@ -33,6 +34,7 @@ static int d_expunge(int, int);
static int d_copy(int, int);
static int d_del(int, int);
static int d_rename(int, int);
+static int d_exec(int, struct buffer *, const char *, const char *, ...);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
@@ -483,14 +485,10 @@ reaper(int signo __attribute__((unused))
int
d_shell_command(int f, int n)
{
- char command[512], fname[MAXPATHLEN], buf[BUFSIZ], *bufp, *cp;
- int infd, fds[2];
- pid_tpid;
- struct sigaction olda, newa;
+ char command[512], fname[MAXPATHLEN], *bufp;
struct buffer *bp;
struct mgwin*wp;
- FILE*fin;
- char sname[NFILEN];
+ char sname[NFILEN];
bp = bfind(*Shell Command Output*, TRUE);
if (bclear(bp) != TRUE)
@@ -506,68 +504,125 @@ d_shell_command(int f, int n)
bufp = eread(! on %s: , command, sizeof(command), EFNEW, sname);
if (bufp == NULL)
return (ABORT);
- infd = open(fname, O_RDONLY);
- if (infd == -1) {
+
+ if (d_exec(0, bp, fname, sh, -c, command, NULL) != TRUE)
+ return (ABORT);
+
+ if ((wp = popbuf(bp, WNONE)) == NULL)
+ return (ABORT); /* XXX - free the buffer?? */
+ curwp = wp;
+ curbp = wp-w_bufp;
+ return (TRUE);
+}
+
+/*
+ * Pipe input file to cmd and insert the command's output in the
+ * given buffer. Each line will be prefixed with the given
+ * number of spaces.
+ */
+static int
+d_exec(int space, struct buffer *bp, const char *input, const char *cmd, ...)
+{
+ char buf[BUFSIZ];
+ va_list ap;
+ struct sigaction olda, newa;
+ char**argv = NULL, *cp;
+ FILE*fin;
+ int fds[2] = { -1, -1 };
+ int infd = -1;
+ int ret = (ABORT), n;
+ pid_tpid;
+
+ if (sigaction(SIGCHLD, NULL, olda) == -1)
+ return (ABORT);
+
+ /* Find the number of arguments. */
+ va_start(ap, cmd);
+ for (n = 2; va_arg(ap, char *) != NULL; n++)
+ ;
+ va_end(ap);
+
+ /* Allocate and build the argv. */
+ if ((argv = calloc(n, sizeof(*argv))) == NULL) {
+ ewprintf(Can't allocate argv : %s, strerror(errno));
+ goto out;
+ }
+
+ n = 1;
+ argv[0] = (char *)cmd;
+ va_start(ap, cmd);
+ while ((argv[n] = va_arg(ap, char *)) != NULL)
+ n++;
+ va_end(ap);
+
+ if (input == NULL)
+ input = /dev/null;
+
+ if ((infd = open(input, O_RDONLY)) == -1) {
ewprintf(Can't open input file : %s, strerror(errno));
- return (FALSE);
+ goto out;
}
+
if (pipe(fds) == -1) {
ewprintf(Can't create pipe : %s, strerror(errno));
- close(infd);
- return (FALSE);
+ goto out;
}
newa.sa_handler = reaper;
newa.sa_flags = 0;
- if (sigaction(SIGCHLD, newa, olda) == -1) {
- close(infd);
- close(fds[0]);
- close(fds[1]);
- return (ABORT);
+ if (sigaction(SIGCHLD, newa, NULL) == -1)
+ goto out;
+
+ if ((pid = fork()) == -1) {
+ ewprintf(Can't fork);
+ goto out;
}
- pid = fork();
+
switch (pid) {
- case -1:
- ewprintf(Can't fork);
- return (ABORT);
- case 0:
+ case 0: /* Child */
close(fds[0]);
dup2(infd, STDIN_FILENO);
dup2(fds[1], STDOUT_FILENO);
dup2(fds[1], STDERR_FILENO);
- execl(/bin/sh, sh, -c, bufp, (char *)NULL);
+ if (execvp(argv[0], argv) == -1)
+ printf(cmd error: %s, strerror(errno));
exit(1);
break;
- default:
+ default: /* Parent */
close(infd);
close(fds[1]);
- fin = fdopen(fds[0], r);
- if (fin == NULL)/* r is surely a valid mode! */
- panic(can't happen);
+ infd = fds[1] = -1;
+ if ((fin =
Re: [PATCH] dired mg patch
On Sat, Aug 27, 2011 at 08:51:08PM +0300, Henri Kemppainen wrote:
The warpdot() has at least one issue. It leads to
segfaults if you try to open a directory like (BCD).
[.. diff ..]
ed
mg doesn't segfault.
Your fix is wrong, and only works by chance. If you craft
a directory with enough spaces in its name, it'll segfault
again. And you cannot fix this in warpdot, it's the wrong
place. The bug is in dired_(), which should abort if the
command fails.
The proper fix is to escape shell metacharacters.
This diff works with all kind of crazy filenames like.
XXX(){}[]'\;|?!$^~LOL
((())
(!^*~
I've looked at the latest diff you've posted on tech@.
The funmap() keybindings are relevant.
There's already strspn(), why write your own
substitute for warpdot() ?
You're also returning by supplying the function inside.
This makes the code less readable.
Lastly, the changes you made in dired_() seem
overcomplicated to me.
You're iterating twice for lforw() and d_warpdot().
In the while() loop, we can already locate the .. and
we just need to lforw() there using the warp variable.
This seems redundant to me.
Here's the diff + my fix for the PR opened by Nima.
Index: src/usr.bin/mg/dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- src/usr.bin/mg/dired.c 23 Jan 2011 00:45:03 - 1.48
+++ src/usr.bin/mg/dired.c 28 Aug 2011 11:00:05 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -165,8 +193,12 @@ dired_init(void)
funmap_add(d_findfile, dired-find-file);
funmap_add(d_ffotherwindow, dired-find-file-other-window);
funmap_add(d_del, dired-flag-file-deleted);
+
Re: [PATCH] dired mg patch
The warpdot() has at least one issue. It leads to segfaults if you try to open a directory like (BCD). [.. diff ..] ed mg doesn't segfault. Your fix is wrong, and only works by chance. If you craft a directory with enough spaces in its name, it'll segfault again. And you cannot fix this in warpdot, it's the wrong place. The bug is in dired_(), which should abort if the command fails. The proper fix is to escape shell metacharacters. That is one (ugly, if you will) way to fix the problem that makes it impossible to open files with funny names. That, however, does not fix the segfaults, because if sh or ls fail for any reason, you'll again adjust the dot offset based on the char array which does not reflect reality. The metacharacter issue is also a bug not related to the warpping we're doing, and is probably best fixed in a separate diff. There's already strspn(), why write your own substitute for warpdot() ? Because llength() is the correct way to determine where a line ends in mg. Notably, lines may embed NUL bytes, and they're not required to terminate with one. Checking the length also makes sure you don't try to read an empty line (which may or may be a NULL pointer, I don't care). You're also returning by supplying the function inside. This makes the code less readable. I'm just returning (TRUE) because the function can never fail. You could also decide that it's appropriate to return (FALSE) if the 9th field cannot be located, but this doesn't change how the assignment should be done. The actual processing is done through pointers on the stack because we cannot do it via the global pointer (curwp), for the reasons I outlined in my previous message. Lastly, the changes you made in dired_() seem overcomplicated to me. If you prefer interleaving unrelated operations, be my guest. After all, it's not my choice. I just hope I needn't touch such code. You're iterating twice for lforw() and d_warpdot(). If I wasn't already sick of this thread, I'd ask you to quote the exact lines of code to show where I'm iterating twice. Because I don't see it. In the while() loop, we can already locate the .. and we just need to lforw() there using the warp variable. This seems redundant to me. Yes, we could, and I explained why I changed that (which is 1: giving warpdot only a pointer to char array is wrong, and 2: the code was interleaved with other things, namely the reading of ls output, and the closing of the pipe). There's a third reason, which is the misuse of strrchr to extract the filename. So I cleaned up the problematic bunch and fixed the segfault. Your warpdot() works differently and doesn't quite conform to style. You're assigning a value to a variable without checking if this is correct or not. This style is hard to read according to me. Bullshit. It's not a matter of style; it's a question It's a matter of style. The way you're designing the loops and the functions look needlessly complicated to me, and it's a bit hard to follow. God. Should forwchar() also return a dot offset and line pointer and let the caller assign them and decide how to handle the exception when they point out of bounds? What if this is a showstopper -- should the caller then return these two values on top of its own return value, and let the caller of the caller decide what to do? We might as propagate all errors to main(). Do you want that 20k line diff? of whether or not an error can happen, and what to do when it happens. In this case, if there was an error to handle, we could return zero, which will always be a legal value (since we are assigning to an int which is zero if the line is empty). Warpdot() is meant to find the 9th column, if it can't, then something's wrong. It should return -1 to indicate failure. To be safe, doto should be set to 0. See above.
Re: [PATCH] dired mg patch
This is a slightly modified diff from Henri's latest one.
It fixes the issue that if a filename has a space at the start of
it, the point will stay in the first character column and not jump
to the first non ' ' character in the filename.
However, it does seem to expose a bug in dired, that if a filename
has a space at the start of it, mg doesn't find it if you try and
open it. mg gives a message of (New File) and creates and empty
buffer. But in the mean time, I think this change does make this
diff more correct.
-lum
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 29 Aug 2011 04:06:16 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(struct line *, int *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -165,8 +193,12 @@ dired_init(void)
funmap_add(d_findfile, dired-find-file);
funmap_add(d_ffotherwindow, dired-find-file-other-window);
funmap_add(d_del, dired-flag-file-deleted);
+ funmap_add(d_forwline, dired-next-line);
funmap_add(d_otherwindow, dired-other-window);
+ funmap_add(d_backline, dired-previous-line);
funmap_add(d_rename, dired-rename-file);
+ funmap_add(d_backpage, dired-scroll-down);
+ funmap_add(d_forwpage, dired-scroll-up);
funmap_add(d_undel, dired-unflag);
maps_add((KEYMAP *)diredmap, dired);
dobindkey(fundamental_map, dired, ^Xd);
@@ -592,6 +624,58 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(struct line *dotp, int *doto)
+{
+ char *tp = dotp-l_text;
+ int off = 0, field = 0, len;
+
+ /*
+* Find the
Re: [PATCH] dired mg patch
Here is a modified diff, the changes revolve around
the d_warpdot function.
Logan has already tested this. Any other test's/oks?
-lum
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 27 Aug 2011 06:54:46 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -592,6 +620,73 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(char *l_text)
+{
+ char *track;
+ int col = 0;
+ int i = 0;
+
+ track = l_text;
+
+ if (track == NULL)
+ return (-1);
+
+ while (*track != '\0') {
+ if((i = strspn(track, )) 0) {
+ track += i;
+ if (++col == 9)
+ break;
+ }
+ track++;
+ }
+ return (track - l_text);
+}
+
+static int
+d_forwpage(int f, int n)
+{
+ forwpage(f | FFRAND, n);
+ if (d_warpdot(curwp-w_dotp-l_text) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+ return TRUE;
+}
+
+static int
+d_backpage (int f, int n)
+{
+ backpage(f | FFRAND, n);
+ if (d_warpdot(curwp-w_dotp-l_text) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+ return TRUE;
+}
+
+static int
+d_forwline (int f, int n)
+{
+ forwline(f | FFRAND, n);
+ if (d_warpdot(curwp-w_dotp-l_text) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto =
Re: [PATCH] dired mg patch
Logan has already tested this. Any other test's/oks?
Is there a good reason to all those if/else blocks which
call d_warpdot twice if it succeeds? I don't see one. At
the very least, I'd remove the redundant call. But we can do
better: just be sane and default to zero on error. In the long
run, the error could be removed entirely by making dired behave
like the rest of mg -- that is, abort somewhere up the stream
if lalloc fails. This way the rest of the functions will never
have to worry about a NULL ltext.
So here's my version. I also removed an unnecessary variable,
added some const in the mix, corrected a minor whitespace nit,
added a comment and renamed some variables (sorry!).
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 27 Aug 2011 12:32:30 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(const char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -592,6 +620,62 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(const char *l_text)
+{
+ const char *tp = l_text;
+ int field = 0;
+
+ if (tp == NULL)
+ return 0;
+
+ /*
+* Find the byte offset to the (space-delimited) filename
+* field in formatted ls output.
+*/
+ while (*tp != '\0') {
+ if (*tp == ' ') {
+ tp += strspn(tp, );
+ if (++field == 9)
+ break;
+ }
+ tp++;
+ }
+ return (tp - l_text);
+}
+
+static int
+d_forwpage(int f, int n)
+{
+ forwpage(f | FFRAND, n);
+
Re: [PATCH] dired mg patch
In the long run, the error could be removed entirely by making
dired behave like the rest of mg -- that is, abort somewhere up
the stream if lalloc fails. This way the rest of the functions
will never have to worry about a NULL ltext.
Actually, this seems to be the case already. dired calls addline,
which never adds a broken line to the buffer. dired_() on the
other hand has a buffer on the stack, and if that didn't work,
we'd never reach warpdot. The NULL check isn't needed. New version.
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 27 Aug 2011 13:04:49 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(const char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -592,6 +620,59 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(const char *l_text)
+{
+ const char *tp = l_text;
+ int field = 0;
+
+ /*
+* Find the byte offset to the (space-delimited) filename
+* field in formatted ls output.
+*/
+ while (*tp != '\0') {
+ if (*tp == ' ') {
+ tp += strspn(tp, );
+ if (++field == 9)
+ break;
+ }
+ tp++;
+ }
+ return (tp - l_text);
+}
+
+static int
+d_forwpage(int f, int n)
+{
+ forwpage(f | FFRAND, n);
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+ return (TRUE);
+}
+
+static int
+d_backpage (int f, int n)
+{
+ backpage(f | FFRAND, n);
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+ return (TRUE);
+}
+
+static int
Re: [PATCH] dired mg patch
Both diffs you submitted work. However, there are some changes that I don't quite agree on. Assigning the value of a function directly to w_doto without checking the return value don't seem right to me. The idea of removing the error to behave like the rest of mg would lead to a brittle design. It's like assuming errors can only happen once. It makes code faster, but later changes could cause subtle bugs that could be hard to track IMHO.
Re: [PATCH] dired mg patch
The warpdot() has at least one issue. It leads to
segfaults if you try to open a directory like (BCD).
Try mkdir \(BCD\) and then reading the contents
of it.
Here's what I get:
drwxr-xr-x 2 root wheel 512 Aug 25 01:09 sh: syntax error: `('
unexpected-rw-r--r-- 1 root wheel 0 Aug 24 18:39 (x
Segmentation fault (core dumped) 0 Aug 27 20:01 .
# -rw-r--r-- 1 root wheel 4 Aug 24 18:40 .(x
Here's one of my diffs:
Index: src/usr.bin/mg/dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- src/usr.bin/mg/dired.c 23 Jan 2011 00:45:03 - 1.48
+++ src/usr.bin/mg/dired.c 27 Aug 2011 15:59:07 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -592,6 +620,83 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(char *l_text)
+{
+ char *track, *anchor;
+ int col = 0;
+ int i = 0;
+ anchor = NULL;
+ track = l_text;
+
+ while (*track != '\0') {
+ if((i = strspn(track, )) 0) {
+ track += i;
+ if (++col == 9) {
+ anchor = track;
+ break;
+ }
+ }
+ track++;
+ }
+ if (anchor == NULL)
+ return (-1);
+ else
+ return (track - l_text);
+}
+
+static int
+d_forwpage(int f, int n)
+{
+ int i;
+
+ forwpage(f | FFRAND, n);
+ if ((i = d_warpdot(curwp-w_dotp-l_text)) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = i;
+ return TRUE;
+}
Re: [PATCH] dired mg patch
The idea of removing the error to behave like the rest of mg would lead to a brittle design. It's like assuming errors can only happen once. It makes code faster, but later changes could cause subtle bugs that could be hard to track IMHO. Quite the opposite, in fact. The idea is to catch errors where and when they happen (i.e. as early as possible), and handle them appropriately, so they do not propagate. This means there are very few places where error checks are needed, which makes it easier to verify these checks are correct. This is no different from designing a function to return as early as possible if, say, a malloc fails. If you don't do this and instead let the errors propagate, all the basic functions will be littered with checks for brokennes that they shouldn't need to deal with in the first place. With such a litter, it'll be easy to break one of these checks. Why should dired behave different from the rest of mg, when it uses the same buffer handling code anyway?
Re: [PATCH] dired mg patch
The warpdot() has at least one issue. It leads to
segfaults if you try to open a directory like (BCD).
New diff below. I wanted to make d_warpdot behave exactly
like the rest of mg functions in that it'd take the two standard
int (f, n) arguments, and dereference curwp-* to do its job.
This was sort of a dead end, since curwp is only set after
everything's been done and the various places that call
dired_() have also called showbuffer() (or the alternative).
All of these would've needed an additional function call, or
a lot of restructuring.
Instead I made warpdot take the line pointer (dotp) and
a pointer to doto, giving it all it needs to operate on
the line mostly like the rest of mg. Now it does the usual
end-of-line check by comparing doto to llength(). Looking
for the NUL byte while iterating over the string was never
the right approach for l_text.
These changes mean dired_() can no longer do horrible evil
like computing the dot offset based on an internal char array
which may or may not reflect the reality of what's actually in
the buffer. This is what caused the segfaults you noticed.
It follows that the first entry after .. logic is no longer
interleaved with a bunch of unrelated stuff. Now it's done
in a separate step after all the ls output has been read.
I also noticed some missing names in the list of keybindings,
and added corresponding funmap_add lines.
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 27 Aug 2011 22:32:36 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(struct line *, int *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -165,8 +193,12 @@ dired_init(void)
Re: [PATCH] dired mg patch
Mark Lumsden contributed code that makes
it faster. I use his suggestions as well for
other sections of the diff.
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 26 Aug 2011 06:14:57 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -592,6 +620,80 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(char *l_text)
+{
+ char *track;
+ int col = 0;
+ int ws = 0;
+
+ track = l_text;
+ while (track != NULL track - l_text = strlen(l_text)) {
+ if((ws = strspn(track, )) 0) {
+ track += ws;
+ if (++col == 9)
+ break;
+ }
+ track++;
+ }
+ if (track == NULL)
+ return (-1);
+ else
+ return (track - l_text);
+}
+
+static int
+d_forwpage(int f, int n)
+{
+ int loc;
+
+ forwpage(f | FFRAND, n);
+ if ((loc = d_warpdot(curwp-w_dotp-l_text)) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = loc;
+ return TRUE;
+}
+
+static int
+d_backpage (int f, int n)
+{
+ int loc;
+
+ backpage(f | FFRAND, n);
+ if ((loc = d_warpdot(curwp-w_dotp-l_text)) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = loc;
+ return TRUE;
+}
+
+static int
+d_forwline (int f, int n)
+{
+ int loc;
+
+ forwline(f | FFRAND, n);
+ if ((loc = d_warpdot(curwp-w_dotp-l_text)) == -1)
+ curwp-w_doto = 0;
+
Re: [PATCH] dired mg patch
Hi,
Jasper@ noticed warnings when compiling
on -current.
This should fix it.
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 23 Aug 2011 16:30:06 -
@@ -36,6 +36,11 @@ static intd_rename(int, int);
static int d_shell_command(int, int);
static int d_create_directory(int, int);
static int d_makename(struct line *, char *, size_t);
+static int d_warpdot(char *);
+static int d_forwpage(int, int);
+static int d_backpage(int, int);
+static int d_forwline(int, int);
+static int d_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndefDIRED_XMAPS
#defineNDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -592,6 +620,75 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(char *l_text)
+{
+ char *track, *anchor = NULL;
+ int col = 0;
+
+ track = l_text;
+ while (track != NULL track - l_text = strlen(l_text)) {
+ if(strspn(track, ) 0) {
+ track += strspn(track, );
+ col++;
+ if (col == 9) {
+ anchor = track;
+ break;
+ }
+ }
+ else
+ track++;
+ }
+ if (anchor == NULL)
+ return (-1);
+ else
+ return (anchor - l_text);
+}
+
+static int
+d_forwpage(int f, int n)
+{
+ forwpage(f | FFRAND, n);
+ if (d_warpdot(curwp-w_dotp-l_text) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+ return TRUE;
+}
+
+static int
+d_backpage (int f, int n)
+{
+ backpage(f | FFRAND, n);
+ if (d_warpdot(curwp-w_dotp-l_text) == -1)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+ return TRUE;
+}
+
+static int
+d_forwline (int f, int n)
+{
+ forwline(f | FFRAND, n);
+
Re: [PATCH] dired mg patch
On Thu, Aug 18, 2011 at 08:30:02AM -0400, Loganaden Velvindron wrote:
I used a function for warping the dot. This
makes the diff simpler.
Since kjell@ has slacked out; any objections to committing this revision of
the diff?
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.48
diff -u -p -r1.48 dired.c
--- dired.c 23 Jan 2011 00:45:03 - 1.48
+++ dired.c 18 Aug 2011 12:23:47 -
@@ -36,6 +36,11 @@ static int d_rename(int, int);
static intd_shell_command(int, int);
static intd_create_directory(int, int);
static intd_makename(struct line *, char *, size_t);
+static intd_warpdot(char *);
+static intd_forwpage(int, int);
+static intd_backpage(int, int);
+static intd_forwline(int, int);
+static intd_backline(int, int);
static void reaper(int);
extern struct keymap_s helpmap, cXmap, metamap;
@@ -57,15 +62,15 @@ static PF dirednul[] = {
static PF diredcl[] = {
reposition, /* ^L */
d_findfile, /* ^M */
- forwline, /* ^N */
+ d_forwline, /* ^N */
rescan, /* ^O */
- backline, /* ^P */
+ d_backline, /* ^P */
rescan, /* ^Q */
backisearch,/* ^R */
forwisearch,/* ^S */
rescan, /* ^T */
universal_argument, /* ^U */
- forwpage, /* ^V */
+ d_forwpage, /* ^V */
rescan, /* ^W */
NULL/* ^X */
};
@@ -77,7 +82,7 @@ static PF diredcz[] = {
rescan, /* ^] */
rescan, /* ^^ */
rescan, /* ^_ */
- forwline, /* SP */
+ d_forwline, /* SP */
d_shell_command,/* ! */
rescan, /* */
rescan, /* # */
@@ -99,9 +104,9 @@ static PF diredc[] = {
};
static PF diredn[] = {
- forwline, /* n */
+ d_forwline, /* n */
d_ffotherwindow,/* o */
- backline, /* p */
+ d_backline, /* p */
rescan, /* q */
d_rename, /* r */
rescan, /* s */
@@ -116,13 +121,32 @@ static PF direddl[] = {
d_undelbak /* del */
};
+static PF diredbp[] = {
+ d_backpage /* v */
+};
+
+static PF dirednull[] = {
+ NULL
+};
+
#ifndef DIRED_XMAPS
#define NDIRED_XMAPS0 /* number of extra map sections */
#endif /* DIRED_XMAPS */
-static struct KEYMAPE (6 + NDIRED_XMAPS + IMAPEXT) diredmap = {
- 6 + NDIRED_XMAPS,
- 6 + NDIRED_XMAPS + IMAPEXT,
+static struct KEYMAPE (1 + IMAPEXT) d_backpagemap = {
+ 1,
+ 1 + IMAPEXT,
+ rescan,
+ {
+ {
+ 'v', 'v', diredbp, NULL
+ }
+ }
+};
+
+static struct KEYMAPE (7 + NDIRED_XMAPS + IMAPEXT) diredmap = {
+ 7 + NDIRED_XMAPS,
+ 7 + NDIRED_XMAPS + IMAPEXT,
rescan,
{
#ifndef NO_HELP
@@ -138,6 +162,10 @@ static struct KEYMAPE (6 + NDIRED_XMAPS
CCHR('L'), CCHR('X'), diredcl, (KEYMAP *) cXmap
},
{
+ CCHR('['), CCHR('['), dirednull, (KEYMAP *)
+ d_backpagemap
+ },
+ {
CCHR('Z'), '+', diredcz, (KEYMAP *) metamap
},
{
@@ -592,6 +620,75 @@ d_makename(struct line *lp, char *fn, si
return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE);
}
+static int
+d_warpdot(char *l_text)
+{
+ char *track, *anchor = NULL;
+ int col = 0;
+
+ track = l_text;
+ while (track != NULL track - l_text = strlen(l_text)) {
+ if(strspn(track, ) 0) {
+ track += strspn(track, );
+ col++;
+ if (col == 9) {
+ anchor = track;
+ break;
+ }
+ }
+ else
+ track++;
+ }
+ if (anchor == NULL)
+ return (NULL);
+ else
+ return (anchor - l_text);
+}
+
+static int
+d_forwpage(int f, int n)
+{
+ forwpage(f | FFRAND, n);
+ if (d_warpdot(curwp-w_dotp-l_text) == NULL)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+ return TRUE;
+}
+
+static int
+d_backpage (int f, int n)
+{
+ backpage(f | FFRAND, n);
+ if (d_warpdot(curwp-w_dotp-l_text) == NULL)
+ curwp-w_doto = 0;
+ else
+ curwp-w_doto = d_warpdot(curwp-w_dotp-l_text);
+
Re: [PATCH] dired mg patch
The 2nd diff which was posted was tested by Nima Hoda. The 3rd diff is mostly a cosmetic change to make the diff less redundant. I contacted other mg users. Hopefull, they'll reply soon.
