marlam / msmtp

SMTP client with sendmail compatible interface
https://marlam.de/msmtp
GNU General Public License v3.0
176 stars 36 forks source link

msmtpq as sendmail piping mail through pager when used with mutt-ical.py #143

Closed Konfekt closed 7 months ago

Konfekt commented 10 months ago

When using Msmtpq instead of Msmtp, this command

https://github.com/marvinthepa/mutt-ical/blob/cba522e6de66d34d4e215fdcd0707e21f7970562/mutt-ical.py#L224

seems to send the mail through a pager (maybe of interest that $LESSOPEN = | lesspipe.sh %s using bat at times) , so that the lines in the mail file in ~/.msmtp.queue are all numbered. With Msmtp this mutt-ical.py (used for answering appointment requests under Mutt) works fine. Since this seems rather odd to me, any explications are welcome.

balejk commented 7 months ago

Have you modified the script in any way?

Have you tried unsetting LESSOPEN and testing if the issue persists?

From what I can tell, the script just pipes the message to the sendmail program -- have you tried pipeing some test message to msmtpq manually on the shell? Does that add the numbers too?

Do you have the latest version of msmtpq? Have you tried looking at its source code for clues?

Konfekt commented 7 months ago

Have you modified the script in any way? Do you have the latest version of msmtpq? Have you tried looking at its source code for clues?

It comes with the package msmtp on Opensuse and the only difference to that in this repo was the first line using #!/bin/bash instead of #!/usr/bin/env bash

Have you tried unsetting LESSOPEN and testing if the issue persists?

Yes, the issue persists

have you tried pipeing some test message to msmtpq manually on the shell? Does that add the numbers too?

I use msmtpq to send every message and they are put into ~/.msmtpqueue without line numbers

However, this script adds them and the mail file reads

  14   --===============6219478304493419352==
  15   Content-Transfer-Encoding: 7bit
  16   Content-Type: text/calendar; charset="utf-8"; method="REPLY"
  17   MIME-Version: 1.0
  18   
  19   BEGIN:VCALENDAR
balejk commented 7 months ago

Enno, 2024-04-09T13:51:02-07:00:

Have you modified the script in any way? Do you have the latest version of msmtpq? Have you tried looking at its source code for clues?

It comes with the package msmtp on Opensuse and the only difference to that in this repo was the first line using #!/bin/bash instead of #!/usr/bin/env bash

I meant the Python script in the first question.

To hopefully help narrow down the cause, I suggest stripping away as much as possible from both components. If you believe the issue is more likely to originate from msmtpq, I would start by replacing the msmtpq script by something like the following:

#!/bin/bash

cat > /tmp/test.mail

since this line (see line 537 in msmtpq) seems to be the only thing that writes the message file. Try to send a message via the Python script and then check if /tmp/test.mail has the message with line numbers or not.

If the former, the message must already contain the numbers once it reaches msmtpq. Which would however be strange since you say it works with plain msmtp, nor do I see anything suspicious in the Python script. I also don't see why msmtpq would work with plain mutt in this case.

Just some random additional ideas: any chance you are perhaps somehow somewhere symlinking cat to bat or something like that? Or that you somehow force -n for cat?

Konfekt commented 7 months ago

Thank you for your assistance. The problem was in the python script indeed as replacing the script by a cat command revealed. Apparently the sendmail command, either string or function reference, confused the script. In line 222 there was

Incompatible types in assignment (expression has type "str | None", target has type "str | Header")

once commented out, there was no more paging.

The crucial fix seems to be

sendmail = lambda: arg.split()  # If -s is provided, override sendmail with a lambda that returns the command

instead of

sendmail = arg.split()

See marvinthepa/mutt-ical@70b3548 (#20) for further details

balejk commented 7 months ago

Enno, 2024-04-10T06:32:33-07:00:

The crucial fix seems to be

sendmail = lambda: arg.split()  # If -s is provided, override sendmail with a lambda that returns the command

instead of

sendmail = arg.split()

See, this is what I meant when I asked whether you have modified the script in any way. The version you referenced in your first post did not have this and only now I see that you added it in your first PR for the script's repository.

See marvinthepa/mutt-ical@70b3548 (#20) for further details

Just a tip: you will probably want to make your editor behave and not optimize the code for you, at least when you are just making changes to existing code. Because from this commit, it's not really obvious at all at the first look what are the actual changes made by you and what are by-products of the optimization. In other words, the diff is very noisy and I had a hard time finding the change you describe, even though it should be just a one line change. If you want to have those optimizations too, they should be put in a separate commit.

Also a heads-up: it seems that the repository was last active two years ago, so it's likely that nobody will respond to or merge your PRs. But some other users of the script might find your work useful or take inspiration from it at least.

Anyway, I'm glad that you were able to solve the problem :-)

Konfekt commented 7 months ago

Yes, to use msmtpq I had to set the sendmail command by hand. However, it was not evident to me that

    if opt == '-s':
            sendmail = arg.split()
    ...
    if not sendmail:
        sendmail = sendmail()

would send it through some pager. It still is not entirely, but at least the error is fixed.

Indeed, the commits are noisy. Sometimes patience to prepare everything for mutual review is limited when there's no reply for years and one ends up using one's own fork