The quote patch includes this change to mutt_body_handler(), in handler.c: - else if (option(OPTREFLOWTEXT) && ascii_strcasecmp ("flowed", mutt_get_parameter ("format", b->parameter)) == 0) + else if (option(OPTREFLOWTEXT) && ascii_strcasecmp ("flowed", mutt_get_parameter ("format", b->parameter)) == 0 && !s->prefix) That's wrong. That line should not be modified; the extra condition "&& !s->prefix" should not be added. It's clearly wrong just from reading code, and it causes two symptoms that I know of. Excising that wrongly added condition fixes the two symptoms. Ideally, the author of that patch would fix it. Otherwise, the FreeBSD port could patch the patch or not use the patch.
(In reply to Kurt Hackenberg from comment #0) Hi Kurt, Can you provide the exact name of the patch you are referring to? Thanks!
The patch file is mutt-2.2.10.vvv.quote. The port configuration has an option "quote patch", which includes that patch file and another, patch-1.13.0.vvv.initials. Symptoms of this bug: 1. Even when the patch file's two features are not enabled, when Mutt quotes a flowed text message into a flowed text reply, it removes trailing spaces from the quoted text, and so converts it to fixed text. 2. When $quote_quoted is yes, lines that start with any character in $quote_regexp are quoted with another copy of that character. That's wrong for flowed text, which RFC 3676 says must be quoted with '>'.
(In reply to Kurt Hackenberg from comment #2) I can't find that file anywhere in the ports collection. Where's that patch exactly?
It's downloaded from http://www.mutt.org.ua/. See /usr/ports/mail/mutt/Makefile, variables PATCH_SITES QUOTE_PATCH_*
I'd rather just drop the patch all together. The less patches the better.
I wouldn't mind if that patch were dropped. I'm just one user though. Do you know whether it's popular? Seems unlikely...
(In reply to Kurt Hackenberg from comment #6) I don't know how popular it is. I don't use mutt. However, being this an optional patch, isn't it OK for you to opt out in make configure? Cheers.
It is an option to disable it but it's on by default. However, given that it might be breaking something it could be a good idea to at the very least make it non-default. I haven't checked to make sure it wasn't breaking something I'm just taking Kurt's word for it. Given that the patch isn't maintained even though :va is distributing it vs :vvv that has apeared to have abandoned it 3 years ago an even better reason to drop it. The less patches the better for the port for maintainability. It's only there for legacy reasons.
Optional or not, the bug I reported is unacceptable. It uses text_plain_handler(), the function for format=fixed, to handle quoting of messages that are text/plain format=flowed. That's a crude hack, blatantly wrong, destructive. It may cause more problems than the two I've noticed. That hack was probably done to apply the patch's quoting features to flowed text as well as fixed, and that idea is wrong. For flowed text, the quoting mechanism is defined by RFC 3676, and may not be changed. As it is now, the patch could generated quoted text that will not be understood by other mail readers. The patch should be either repaired or dropped. The patch adds two optional small features that modify the string of '>' and space used to quote a message in a reply. It does not add those features to the Mutt documentation, so they're probably not widely known. I use Mutt, and didn't know those features existed. I found all this by debugging a symptom. Also, I agree with Derek that it's best to hold down the number of patches. I say, drop the patch.
While this might be extreme what about dropping all the feature patches. While you are one user... Do you care about any of the other feature patches?
I didn't know any of them existed until the other day, when I tracked down this bug. I've looked briefly at some of them, but not all; haven't tried any but the quote stuff. Note that the quote patch and the initials patch are packaged together in the single configuration option "quote patch". Opinions so far, based on not much: quote, initials, greeting: these strike me as minor and only marginally useful. I don't plan to use them, and wouldn't miss them. smart date, reverse reply: probably the same, but I'd want to look at them more. ifdef: looks handy, if it is what I think it is. Needs more attention. maildir mtime: doesn't look like a feature, don't know what it's about. I wouldn't change anything without knowing more.
Created the following https://lists.freebsd.org/archives/freebsd-ports/2023-May/003815.html https://marc.info/?l=mutt-users&m=168330457408909&w=2 Hoping to get some feedback. I think the current plan is the remove quote patch, keep initial patch either way a new option or just keep the QUOTE_PATCH option. Maybe use UPDATING to solicit information for more feedback.
If you drop the quote patch and keep the initials patch, you should rename the port configuration option, to "initials". Calling it "quote" when it has nothing to do with quoting would be confusing.
Created https://reviews.freebsd.org/D39985 for review.
Created attachment 242007 [details] Remove quote and future patch changes Patch from https://reviews.freebsd.org/D39985
LGTM except that we need to bump PORTREVISION since we are changing options. No need to update the patch, I can do it at commit time. Please set the maintainer-approval attachment flag (to +) on patches for ports you maintain to signify approval. Then, I can proceed with the commit :-)
(In reply to Fernando Apesteguía from comment #16) doh always forget about rev. Thanks for the feedback. I plan on waiting till Mon/Tues to move it to +.
(In reply to Derek Schrock from comment #17) Ok, have a good weekend!
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=efe2cf37e62dedefdf9996990f197af13262bb2f commit efe2cf37e62dedefdf9996990f197af13262bb2f Author: Derek Schrock <dereks@lifeofadishwasher.com> AuthorDate: 2023-05-06 18:14:03 +0000 Commit: Fernando Apesteguía <fernape@FreeBSD.org> CommitDate: 2023-05-09 09:38:23 +0000 mail/mutt: Remove QUOTE_PATCH option and future patch removal * Removing QUOTE_PATCH due to logic issues and RFC3676 quoting. * Keep vvv initials patch from QUOTE_PATCH adding INITIALS_PATCH * Add depercation of other patches to be removed in future updates via UPDATING and pkg-message. See PR 271190 [1] comment 12 [2]. * SASL_NONE will now add --with-sasl=no (making portlint happy) * Update context on patches [1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271190 [2] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271190#c12 Context and discussion: https://lists.freebsd.org/archives/freebsd-ports/2023-May/003815.html PR: 271190 Reported by: kh@panix.com Differential Revision: https://reviews.freebsd.org/D39985 mail/mutt/Makefile | 23 ++++++++++++----------- mail/mutt/distinfo | 4 +--- mail/mutt/files/extra-patch-ifdef | 10 +++++----- mail/mutt/files/extra-patch-maildir-mtime | 4 ++-- mail/mutt/files/extra-patch-reverse_reply | 18 +++++++++--------- mail/mutt/files/extra-patch-smartdate | 4 ++-- mail/mutt/files/extra-smime-sender | 10 +++++----- mail/mutt/files/patch-date-conditional | 6 +++--- mail/mutt/files/patch-dgc-deepif | 2 +- mail/mutt/pkg-message (new) | 18 ++++++++++++++++++ 10 files changed, 58 insertions(+), 41 deletions(-)
Committed, Thanks!
Thanks however any reason UPDATING was excluded?
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=429dc21f6687578c0ead0d11e5a87784224f14ae commit 429dc21f6687578c0ead0d11e5a87784224f14ae Author: Fernando Apesteguía <fernape@FreeBSD.org> AuthorDate: 2023-05-10 06:30:00 +0000 Commit: Fernando Apesteguía <fernape@FreeBSD.org> CommitDate: 2023-05-10 06:30:00 +0000 UPDATING: Add entry for mail/mutt Present in the OP's patch, but that I somehow managed to miss. PR: 271190 Fixes: efe2cf37e62d UPDATING | 9 +++++++++ 1 file changed, 9 insertions(+)
(In reply to Derek Schrock from comment #21) Should be fixed now. I don't know what happened sorry :S I didn't even have UPDATING as modified, so it was not that I forgot to add it before committing... Thanks for the heads up!
Created attachment 242143 [details] Diff between current and fixed
Created attachment 242144 [details] Makefile.radio
I built Mutt from the new port, and found a bug: it fails to send through the SMTP server of one mail provider, with these messages: [2023-05-12 22:23:16] SMTP authentication requires SASL [2023-05-12 22:23:17] No authenticators available I found and fixed a bug in the Makefile. Attachment shows diff between current and fixed, for Makefile, output of "make showconfig", and output of "mutt -v". Note that the bug also messes with the options reverse reply and smart date. I also did an experiment: converted OPTIONS_SINGLE to OPTIONS_RADIO, for GSSAPI, HCACHE, and SASL, since they follow that pattern. Removed all explicit references to their NONE options. That seems to work, but needs more testing. Attached that Makefile. I don't know much about port Makefiles; please check what I've done. The port needs to be fixed, though.
Dang. Lets try to address these one at a time... Ok I understand the SASL issue. I think having the --with-sasl might be an issue in cases where it's looking for something outside of LOCALBASE? Having the just do nothing should be fine? -SASL_NONE_CONFIGURE_WITH= sasl=no +SASL_NONE_CONFIGURE_OFF= +SASL_NONE_CONFIGURE_ON= --with-sasl=no Building with each option mutt -v shows + for the given feature and - for all when disabled. I'm not following the issue with the reverse reply and smarte date options. I like the idea of the radio. However, going to leave it out until the port is stable. However, the idea is you unselect the option to make it "none"?
Created https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271395
The --with-sasl stuff -- I wrote those two lines imitating other NONE_CONFIGURE_{ON,OFF} in the Makefile. Depends on exactly how the configure script interprets those options, which I don't really know. Reverse reply and smart date -- I don't know why that happens, but we see that it does. I notice those options surround SASL_CYRUS in OPTIONS_DEFAULT; maybe there's some text processing problem. We see that it's fixed with the fix to SASL_NONE_CONFIGURE. Radio, unselect for none -- I guessed that, and it seems to work for GSSAPI. Needs more testing.