[PATCH] dired mg patch
Based on discussion with mark & nima, I think we can overwrite dired keybindings with the ones that warp the dot to the file name. This also fixes an issue Mark encountereted with filenames that contain spaces. Since this is a huge diff, I need some heavy testing ;-). 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 12 Aug 2011 17:45:41 - @@ -36,6 +36,10 @@ 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_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 +61,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 +81,7 @@ static PF diredcz[] = { rescan, /* ^] */ rescan, /* ^^ */ rescan, /* ^_ */ - forwline, /* SP */ + d_forwline, /* SP */ d_shell_command,/* ! */ rescan, /* " */ rescan, /* # */ @@ -99,9 +103,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 +120,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 +161,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 +619,118 @@ d_makename(struct line *lp, char *fn, si return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE); } +static int +d_forwpage(int f, int n) +{ + char tline[256], *anchor, *last, *col[9]; + int lim = 0, z = 9; + + while (z--) + col[z] = NULL; + forwpage(f | FFRAND, n); + (void) strlcpy(tline, curwp->w_dotp->l_text, sizeof + (tline)); + for ((anchor = strtok_r(tline, " ", &last)); anchor; + (anchor = strtok_r(NULL, " ", &last))) { + if (lim <= 8) + col[lim++] = anchor; + } + col[lim] = NULL; + if (col[8] == NULL || + strstr(curwp->w_dotp->l_text, col[8]) == NULL) + curwp->w_doto = 0; + else { + curwp->w_doto = + strstr(curwp->w_dotp->l_text, col[8]) - + curwp->w_dotp->l_text; + } + return TRUE; +} + +static int +d_backpage (int f, int n) +{ + char tline[256], *anchor, *last, *col[9]; + int lim = 0, z = 9; + + while (z--) + col[z] = NULL; + backpage(f | FFRAND, n); + (void) strlcpy(tline, curwp->w_dotp->l_text, sizeof + (tline)); +
Re: [PATCH] dired mg patch
The previous diff had issues with duplicate colums. e.g 4 drwx-- 3 root wheel 512 Aug 9 02:36 root using strstr() would give location of the first instance of `root'. The following is simpler, and calculates the offset of the 9th column, and also avoids using strtok_r() which requires a lot of care to get done right. Now, I use strspn(). Au usual complaints welcomed ;-) 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 13 Aug 2011 16:33:11 - @@ -36,6 +36,10 @@ 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_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 +61,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 +81,7 @@ static PF diredcz[] = { rescan, /* ^] */ rescan, /* ^^ */ rescan, /* ^_ */ - forwline, /* SP */ + d_forwline, /* SP */ d_shell_command,/* ! */ rescan, /* " */ rescan, /* # */ @@ -99,9 +103,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 +120,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 +161,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 +619,118 @@ d_makename(struct line *lp, char *fn, si return ((lgetc(lp, 2) == 'd') ? TRUE : FALSE); } +static int +d_forwpage(int f, int n) +{ + char *track, *anchor = NULL; + int col = 0; + + forwpage(f | FFRAND, n); + track = curwp->w_dotp->l_text; + while (track != NULL && track - curwp->w_dotp->l_text <= + strlen(curwp->w_dotp->l_text)) { + if (strspn(track, " ") > 0) { + track += strspn(track, " "); + col++; + if (col == 9) { + anchor = track; + break; + } + } + else + track++; + } + if (anchor == NULL) + curwp->w_doto = 0; + else + curwp->w_doto = anchor - curwp->w_dotp->l_text; + return TRUE; +} + +static int +d_backpage (int f, int n) +{ + char *track, *anchor = NULL; + int col = 0; + + backpage(f | FFRAND, n); + track = curwp->w_dotp->l_text;
Re: [PATCH] dired mg patch
I used a function for warping the dot. This makes the diff simpler. 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 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 (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); + return TRUE; +} + +static int +d_forwline (int f, int n) +{ + forwline(f |
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
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.
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
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) + c
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 + curw
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); +} + +sta
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
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). [.. 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. > Here's a snippet from ntpd.c > if ((pw = getpwnam(NTPD_USER)) == NULL) > errx(1, "unknown user %s", NTPD_USER); > When you're designing functions, you should account for > error values as well. -1 is used by most code written > from scratch in base. The ntpd quote is totally irrelevant. When designing functions, you have to account for errors, if such can happen. In library functions, returning an error value is often the only possible solution, because the library does not know how you want to solve the situation. In other places, there may or may not be a way to handle the error immediately. For a simple application, the developer could decide that it's simply not worth it trying to recover from a failed allocation; in this case, he handles it with a wrapper function which simply makes the program quit instantly. In other cases, you have options such as "do nothing", in which case the operation does not complete, but it doesn't cause a crash either. An example: trying to move the dot past the start or end of the buffer in mg would result in a no-op, and other functions do not have to try and verify that the dot is within the legal range. In the addline case, there is a way to handle the error: don't add a line if you can't allocate it. The error is handled in such a way that most of the other functions do not have to worry about it. There are other places where errors are handled "up the stream" likewise. If you still think this is wrong, I could cook you a 20k line diff that makes every single function check that 1) the character we read from stdin isn't EOF, 2) curwp is not null, 3) curwp->w_dotp is not null, 4) curbp is not null, and so on. By now you should realize how ridiculous this is. These errors are (or should be) accounted for up the stream so that none of the functions that actually have to operate on existing buffer or window contents should worry about it. > 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 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). If warpdot was a library function whose callers need to react to the error in different ways, then we'd have to return an error value. Like I said above, the segfault you got has nothing to do with warpdot, and trying to fix it there is wrong.
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) f
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, "
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; + +
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 #include #include +#include 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", 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)) -
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 #include #include +#include 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 */ clo
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 #include #include +#include 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 = fdo