pulb / mailnag

An extensible mail notification daemon
GNU General Public License v2.0
250 stars 32 forks source link

Adds plugin that allows marking IMAP messages as read #215

Closed andia89 closed 3 years ago

andia89 commented 3 years ago

I wrote a small plugin that allows marking IMAP messages as read also on the server upon doing it in mailnag. It required some minor changes to the code base of mailnag, but nothing serious (apart from the fact that the folders cannot be readonly anymore). I think that is a nice addition that solves a small annoyance.

I'm not entirely sure overloading a function like that is the idea of plugins for this app, but I didn't want to rewrite the whole dBus implementation

Solves #214

pulb commented 3 years ago

Hey andia89,

many thanks for your pull request, this feature is very appreciated! :smiley: Though can you please add the feature to the daemon itself instead to a plugin? As you already pointed out, your plugin implementation is a bit hackish as the mark-as-read functionality isn't exposed by the plugin api and hence your plugin accesses/patches daemon internals which can easily change and break the plugin in the future. Besides that I also believe most people want to have their mails marked as read on the imap server by default. If not, we can make this optional in mailnag.cfg.

andia89 commented 3 years ago

All done. That was even easier then :) Ive added a section to mailnag.cfg just in case someone doesn't like the change

pulb commented 3 years ago

Wow, that was lightning fast! :D Awesome, will review and merge it ASAP.

pulb commented 3 years ago

Sorry for not merging your PR yet. I reviewed it already but there are a view changes I don't like. I didn't find the time yet to think about a better solution for those changes unfortunatelly. I'll add some comments for the time being.

pulb commented 3 years ago

Thanks again! Will do some refactoring later

pulb commented 3 years ago

No worries, I fixed all commented issues and refactored some stuff. Will push my changes within the next days :)

pulb commented 3 years ago

I pushed all fixes in commit a124d14be80d3afec8a48e0201e87b7d2fed6abd.

andia89 commented 3 years ago

Thanks for the refactor. It seems to be not working for me though. Messages are not marked as read on my gmail anymore... I will investigate

pulb commented 3 years ago

Hmm It should work in all configuration scenarios. The main difference to your original patch is basically that messages are not marked as read immediately and seperately on every click , but "batch-marked" upon the next mail check on the server.

Am 6. November 2020 11:15:41 MEZ schrieb Andreas Angerer notifications@github.com:

Thanks for the refactor. It seems to be not working for me though. Messages are not marked as read on my gmail anymore... I will investigate

-- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/pulb/mailnag/pull/215#issuecomment-722998272

andia89 commented 3 years ago

@pulb Just saw the hanging comment here: Yes it works, but the delay between marking it as read and it eventually being marked on the server side is a bit annoying when trying to clean up my inbox (at least to me). I "fixed" that for me by adding a self.check_for_mails() at the end of mark_mail_as_read in the file mailnagdaemon.py, like that

def mark_mail_as_read(self, mail_id):
        # Note: ensure_not_disposed() is not really necessary here
        # (the memorizer object is available in dispose()), 
        # but better be consistent with other daemon methods.
        self._ensure_not_disposed()

        self._memorizer.set_to_seen(mail_id)
        self._memorizer.save() 
                self.check_for_mails()

do you think that is a proper fix or to hackish?

pulb commented 3 years ago

Hi Andreas,

I already thought about that workaround too, but came to the conclusion that it isn't a good idea because

  1. check_for_mails() currently does not check idle accounts. (If it would, it would wake up all sleeping idle threads, which all at once would do a further check)
  2. it would perform an unnecessary mail check for accounts that don't support server-side mark-as-read (e.g. pop3)
  3. marking a mail as read from the gnome-shell extension would block/defer further attempts by the user to mark mails as read until the current mail check has been finished (which may take a long time, depending of the number of checked accounts and unread messages)
  4. the "mark all as read" command of the gnome-shell extension does trigger mark-as-read requests for every single mail, so due to the problem stated in 3., marking all mails as read may take... forever.
pulb commented 3 years ago

What would speed up things though would be to decrease the poll interval / idle timeout in the config file.