Hi [2022-02-05 16:53] Philipp Takacs <[email protected]> > Reply to mime messages is an old problem. I have an idea to workaround > or fix this. Instand of adding mime to mhl, repl could also first decode > mime before pipe the mail to mhl. This can be done by mhshow. > > I have attached a patch for this.
Thanks for all the discoussion. I have an updated version of this patch. I belive it's clear that you want a switch for this so I have implemented it. Currently on by default. Also I have an other patch which implements Ralphs suggestion. I'm not sure if the manpage updates are good. Also the tests should be updated. I'm also look for a way to autodetect if -nofixmime should be set. I think the best place would be in nmh_init(). As a disclousure: I work with this patch since 2016. If you implement it or not will not affect me. I thought this was clear from the beginning, but it looks for me that this wasn't clear for everyone in the discussion. Sorry for that. I belive this patch is good and you will benefit from it. Thats is why I have send it to you and discoused why I belive this should be enable by default. But in the end you can decide if and in which way you want this patch. Philipp
From a587df65eafb93bea5cc74053823fd07bc13ea8b Mon Sep 17 00:00:00 2001 From: Philipp Takacs <[email protected]> Date: Sat, 5 Feb 2022 12:21:35 +0100 Subject: [PATCH 1/2] repl: pipe mail through mhshow When reply to a MIME message the mhshow will decode the mime message before mhl will get it to create a reply draft. This makes it more convinient to reply to a MIME message. Problems: * mhshow can be configured to display parts in a graphical programm. This can lead to unexpected behavior on repl (i.e. start plaing musik). * mhshow display string can contain '%l' which leads to a listing prior to displaying content. This listing is contained in the draft. --- man/repl.man | 22 ++++++++ uip/repl.c | 11 +++- uip/replsbr.c | 137 +++++++++++++++++++++++++++++++++++++------------- uip/replsbr.h | 2 +- 4 files changed, 134 insertions(+), 38 deletions(-) diff --git a/man/repl.man b/man/repl.man index 790e19b5..fc1935fc 100644 --- a/man/repl.man +++ b/man/repl.man @@ -51,6 +51,7 @@ all/to/cc/me] .RB [ \-build ] .RB [ \-file .IR msgfile ] +.RB [ \-fixmime " | " \-nofixmime ] .ad .SH DESCRIPTION .B repl @@ -71,6 +72,14 @@ format file (see .IR mh\-format (5) for details). .PP +To decode MIME messages +.B repl +pipe the message through +.IR showmimeproc . +This can be disabled with the +.B \-nofixmime +switch. +.PP If the switch .B \-nogroup is given (it is on by default), then @@ -516,6 +525,7 @@ is checked. ^Editor:~^To override the default editor ^Msg\-Protect:~^To set mode when creating a new message (draft) ^fileproc:~^Program to refile the message +^showmimeproc:~^Program to show non-text (MIME) messages ^mhlproc:~^Program to filter message being replied-to ^whatnowproc:~^Program to ask the \*(lqWhat now?\*(rq questions .fi @@ -542,6 +552,7 @@ is checked. .RB ` \-noquery ' .RB ` \-noatfile ' .RB ` "\-width\ 72" ' +.RB ` \-fixmime ' .fi .SH CONTEXT If a folder is given, it will become the current folder. The message @@ -578,3 +589,14 @@ don't call it since .B repl won't run it. +.PP +The +.RB \-fixmime +switch causes repl to pipe the message through +.IR showmimeproc . +If the +.IR showmimeproc +is configured to display listing prior a mime part, these listing will end up in the draft. +There might also be some unexpected behaviors, when the +.IR showmimeproc +is configured to display the content externel (i.e. open a browser to display html). diff --git a/uip/repl.c b/uip/repl.c index b1ceb1f6..8ed22709 100644 --- a/uip/repl.c +++ b/uip/repl.c @@ -72,6 +72,8 @@ X("noatfile", 0, NOATFILESW) \ X("fmtproc program", 0, FMTPROCSW) \ X("nofmtproc", 0, NFMTPROCSW) \ + X("fixmime", 0, FIXMIMESW) \ + X("nofixmime", 0, NFIXMIMESW) \ #define X(sw, minchars, id) id, DEFINE_SWITCH_ENUM(REPL); @@ -143,6 +145,7 @@ main (int argc, char **argv) bool nedit = false; bool nwhat = false; bool atfile = false; + bool fixmime = true; int fmtproc = -1; char *cp, *cwd, *dp, *maildir, *file = NULL; char *folder = NULL, *msg = NULL, *dfolder = NULL; @@ -354,6 +357,12 @@ main (int argc, char **argv) case NFMTPROCSW: fmtproc = 0; continue; + case FIXMIMESW: + fixmime = true; + continue; + case NFIXMIMESW: + fixmime = false; + continue; } } if (*cp == '+' || *cp == '@') { @@ -469,7 +478,7 @@ try_it_again: } replout (in, msg, drft, mp, outputlinelen, mime, form, filter, - fcc, fmtproc); + fcc, fmtproc, fixmime); fclose (in); { diff --git a/uip/replsbr.c b/uip/replsbr.c index ef4925ae..d2bab72f 100644 --- a/uip/replsbr.c +++ b/uip/replsbr.c @@ -61,7 +61,7 @@ static char *addrcomps[] = { * static prototypes */ static int insert (struct mailname *); -static void replfilter (FILE *, FILE *, char *, int); +static void replfilter (FILE *, FILE *, char *, int, bool); static char *replformataddr(char *, char *); static char *replconcataddr(char *, char *); static char *fix_addresses (char *); @@ -69,7 +69,7 @@ static char *fix_addresses (char *); void replout (FILE *inb, char *msg, char *drft, struct msgs *mp, int outputlinelen, - int mime, char *form, char *filter, char *fcc, int fmtproc) + int mime, char *form, char *filter, char *fcc, int fmtproc, bool fixmime) { int state, i; struct comp *cptr; @@ -241,7 +241,7 @@ finished: if (ferror (out)) adios (drft, "error writing"); - replfilter (inb, out, filter, fmtproc); + replfilter (inb, out, filter, fmtproc, fixmime); } else if (mime && mp) { fprintf (out, "#forw [original message] +%s %s\n", mp->foldpath, m_name (mp->lowsel)); @@ -421,21 +421,63 @@ insert (struct mailname *np) return 1; } +static void +execmhl(char *mhl, char **arglist, int argnum, char *filter, int fmtproc) +{ + char *errstr; + + /* + * We're not allocating the memory for the extra arguments, + * because we never call arglist_free(). But if we ever change + * that be sure to use getcpy() for the extra arguments. + */ + arglist[argnum++] = "-nomoreproc"; + arglist[argnum++] = "-form"; + arglist[argnum++] = filter; + arglist[argnum++] = "-noclear"; + + switch (fmtproc) { + case 1: + arglist[argnum++] = "-fmtproc"; + arglist[argnum++] = formatproc; + break; + case 0: + arglist[argnum++] = "-nofmtproc"; + break; + } + + arglist[argnum++] = NULL; + + execvp (mhl, arglist); + errstr = strerror(errno); + if (write(2, "unable to exec ", 15) < 0 || + write(2, mhlproc, strlen(mhlproc)) < 0 || + write(2, ": ", 2) < 0 || + write(2, errstr, strlen(errstr)) < 0 || + write(2, "\n", 1) < 0) { + advise ("stderr", "write"); + } + _exit(1); +} /* - * Call the mhlproc + * Start the filter pipeline * * This function expects that argument out has been fflushed by the caller. */ static void -replfilter (FILE *in, FILE *out, char *filter, int fmtproc) +replfilter (FILE *in, FILE *out, char *filter, int fmtproc, bool fixmime) { int pid; char *mhl; char *errstr; char **arglist; int argnum; + int mailpipe[2]; + char *mhshow; + char **mhshowargs; + int mshowargnum; if (filter == NULL) return; @@ -447,48 +489,71 @@ replfilter (FILE *in, FILE *out, char *filter, int fmtproc) lseek(fileno(in), 0, SEEK_SET); arglist = argsplit(mhlproc, &mhl, &argnum); + mhshowargs = argsplit(showmimeproc, &mhshow, &mshowargnum); switch (pid = fork()) { case NOTOK: adios ("fork", "unable to"); case OK: - dup2 (fileno (in), fileno (stdin)); - dup2 (fileno (out), fileno (stdout)); - - /* - * We're not allocating the memory for the extra arguments, - * because we never call arglist_free(). But if we ever change - * that be sure to use getcpy() for the extra arguments. - */ - arglist[argnum++] = "-form"; - arglist[argnum++] = filter; - arglist[argnum++] = "-noclear"; - - switch (fmtproc) { - case 1: - arglist[argnum++] = "-fmtproc"; - arglist[argnum++] = formatproc; - break; - case 0: - arglist[argnum++] = "-nofmtproc"; - break; - } - arglist[argnum++] = NULL; + if (fixmime) { + if (pipe(mailpipe) == -1) { + adios("pipe", "can't create pipe"); + } - execvp (mhl, arglist); - errstr = strerror(errno); - if (write(2, "unable to exec ", 15) < 0 || - write(2, mhlproc, strlen(mhlproc)) < 0 || - write(2, ": ", 2) < 0 || - write(2, errstr, strlen(errstr)) < 0 || - write(2, "\n", 1) < 0) { - advise ("stderr", "write"); + switch (pid = fork()) { + case NOTOK: + adios ("fork", "unable to"); + + case OK: + dup2 (fileno (in), fileno (stdin)); + dup2 (mailpipe[1], fileno (stdout)); + close(fileno(in)); + close(fileno(out)); + close(mailpipe[0]); + close(mailpipe[1]); + + /* + * We're not allocating the memory for the extra arguments, + * because we never call arglist_free(). But if we ever change + * that be sure to use getcpy() for the extra arguments. + */ + mhshowargs[argnum++] = "-file"; + mhshowargs[argnum++] = "-"; + mhshowargs[argnum++] = "-concat"; + mhshowargs[argnum++] = NULL; + + execvp (mhshow, mhshowargs); + errstr = strerror(errno); + if (write(2, "unable to exec ", 15) < 0 || + write(2, mhshow, strlen(mhshow)) < 0 || + write(2, ": ", 2) < 0 || + write(2, errstr, strlen(errstr)) < 0 || + write(2, "\n", 1) < 0) { + advise ("stderr", "write"); + } + _exit(1); + default: + dup2 (mailpipe[0], fileno (stdin)); + dup2 (fileno (out), fileno (stdout)); + close(fileno (in)); + close(fileno (out)); + close(mailpipe[0]); + close(mailpipe[1]); + execmhl(mhl, arglist, argnum, filter, fmtproc); + } + } else { + dup2(fileno (in), fileno (stdin)); + dup2(fileno (out), fileno (stdout)); + close(fileno (in)); + close(fileno (out)); + execmhl(mhl, arglist, argnum, filter, fmtproc); } - _exit(1); default: + close(fileno(in)); + close(fileno(out)); if (pidXwait (pid, mhl)) done (1); fseek (out, 0L, SEEK_END); diff --git a/uip/replsbr.h b/uip/replsbr.h index 2379962a..22423ed1 100644 --- a/uip/replsbr.h +++ b/uip/replsbr.h @@ -7,7 +7,7 @@ void replout(FILE *inb, char *msg, char *drft, struct msgs *mp, int outputlinelen, int mime, char *form, char *filter, char *fcc, - int fmtproc); + int fmtproc, bool fixmime); extern short ccto; extern short cccc; -- 2.30.2
From 286d44644c49bfb6c71a559fb3dad15516d9f465 Mon Sep 17 00:00:00 2001 From: Philipp Takacs <[email protected]> Date: Tue, 8 Feb 2022 11:16:48 +0100 Subject: [PATCH 2/2] repl set argv0 of showmimeproc to mhreplyfmt. This allows to have diffrent config for showmimeproc for replies. --- man/repl.man | 25 +++++++++++++------------ uip/replsbr.c | 1 + 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/man/repl.man b/man/repl.man index fc1935fc..3162db8c 100644 --- a/man/repl.man +++ b/man/repl.man @@ -75,7 +75,16 @@ for details). To decode MIME messages .B repl pipe the message through -.IR showmimeproc . +.IR showmimeproc +by default. +To allow diffrent config the +.IR showmimeproc +will be called as +.IR mhreplyfmt . +So the config will be prefixed with mhreplyfmt instand of mhshow. +For example mhreplyfmt-show-text/html (see +.IR mhshow (1) +for details) is used to display a html mail. This can be disabled with the .B \-nofixmime switch. @@ -526,6 +535,8 @@ is checked. ^Msg\-Protect:~^To set mode when creating a new message (draft) ^fileproc:~^Program to refile the message ^showmimeproc:~^Program to show non-text (MIME) messages +^mhreplyfmt-charset-<charset>~^Template for environment to render character sets +^mhreplyfmt-show-<type>*~^Template for displaying contents ^mhlproc:~^Program to filter message being replied-to ^whatnowproc:~^Program to ask the \*(lqWhat now?\*(rq questions .fi @@ -536,6 +547,7 @@ is checked. .IR mhbuild (1), .IR send (1), .IR whatnow (1) +.IR mhshow (1) .PP .I %docdir%/contrib/replaliases .SH DEFAULTS @@ -589,14 +601,3 @@ don't call it since .B repl won't run it. -.PP -The -.RB \-fixmime -switch causes repl to pipe the message through -.IR showmimeproc . -If the -.IR showmimeproc -is configured to display listing prior a mime part, these listing will end up in the draft. -There might also be some unexpected behaviors, when the -.IR showmimeproc -is configured to display the content externel (i.e. open a browser to display html). diff --git a/uip/replsbr.c b/uip/replsbr.c index d2bab72f..477f8814 100644 --- a/uip/replsbr.c +++ b/uip/replsbr.c @@ -519,6 +519,7 @@ replfilter (FILE *in, FILE *out, char *filter, int fmtproc, bool fixmime) * because we never call arglist_free(). But if we ever change * that be sure to use getcpy() for the extra arguments. */ + mhshowargs[0] = "mhreplyfmt"; mhshowargs[argnum++] = "-file"; mhshowargs[argnum++] = "-"; mhshowargs[argnum++] = "-concat"; -- 2.30.2
