Adds plugin that allows marking IMAP messages as read #215

andia89 · 2020-10-02T19:36:12Z

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 · 2020-10-05T17:53:57Z

Hey andia89,

many thanks for your pull request, this feature is very appreciated! 😃 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 · 2020-10-05T18:24:16Z

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

pulb · 2020-10-06T08:36:28Z

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

pulb · 2020-10-31T14:50:52Z

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 · 2020-10-31T14:55:53Z

Mailnag/backends/imap.py

 				for response_part in msg_data:
 					if isinstance(response_part, tuple):
 						try:
 							msg = email.message_from_bytes(response_part[1])
 						except:
 							logging.debug("Couldn't get IMAP message.")
 							continue
-						yield (folder, msg)
+						yield (folder, msg, num.decode("utf-8"))


The list_messages() method is defined in the base interface and implemented in several backends like in this imap backend. list_messages() of all other backends returns (folder, msg) pairs, wheras imap now returns (folder, msg, num) tuples, which breaks the interface (inconsistency).

I see, that is of course an issue. Maybe it would be a way to add some kind of dummy number to the other backends? I don't see any other way of letting mailnag know about this imap specific id though

pulb · 2020-10-31T15:00:36Z

Mailnag/daemon/mailnagdaemon.py

@@ -126,9 +126,22 @@ def shutdown(self):

 	# Part of MailnagController interface
 	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.


This comment should not be removed

Yes, of course

pulb · 2020-10-31T15:01:40Z

Mailnag/daemon/dbus.py

@@ -110,7 +110,7 @@ def _convert_mails(self, mails):
 			d['sender_addr'] = addr				# string (s)
 			d['account_name'] = m.account_name	# string (s)
 			d['id'] = m.id						# string (s)
-
+			d['strID'] = m.strID				# string (s)


Why is strID exposed by the dbus interface?

I think that was a mistake on my side, I think I tested that once to let the gnome-shell extension know about the ID (and open the specific mail directly when clicking on it) in the end it didn't really work. I guess it can be removed

pulb · 2020-10-31T15:02:36Z

Mailnag/backends/imap.py

@@ -225,8 +225,7 @@ def _select_single_folder(self, conn):
 			folder = self.folders[0]
 		else:
 			folder = "INBOX"
-
-		conn.select(f'"{folder}"', readonly = True)
+		conn.select(f'"{folder}"')


Is it really necessary to remove the readonly flag?

The readonly definitely has to go, I mean one could put readonly = False, but that is just a matter of taste

pulb · 2020-10-31T15:25:17Z

Mailnag/daemon/mails.py

 		self.account_name = account.name
 		self.account_id = account.get_id()
 		self.id = id
+		self.strID = strID


I don't like the idea of adding an imap specific mail counter (strID) to the generic mail class. I can't come up with a better solution right now, though.

pulb · 2020-10-31T15:27:03Z

Mailnag/daemon/mailnagdaemon.py

+			return
+		backend = mail.account._get_backend()
+		if type(backend).__name__ == 'IMAPMailboxBackend':
+			mailid = mail.strID


The terms mailID and strID are missleading. This is rather a mail counter.

pulb · 2020-10-31T15:32:24Z

Mailnag/daemon/mailnagdaemon.py

+			self._memorizer.save()
+			return
+		backend = mail.account._get_backend()
+		if type(backend).__name__ == 'IMAPMailboxBackend':


Adding backend specific code here feels hackish. This should be implemented within the backend, which marks the mail as read if it supports it.

pulb · 2020-11-01T15:41:58Z

Thanks again! Will do some refactoring later

pulb · 2020-11-03T11:33:16Z

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

pulb · 2020-11-04T19:45:31Z

I pushed all fixes in commit a124d14.

andia89 · 2020-11-06T10:15:23Z

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 · 2020-11-06T10:26:44Z

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: #215 (comment)

andia89 · 2021-02-05T09:12:22Z

@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 · 2021-02-05T10:18:15Z

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 · 2021-02-05T10:42:55Z

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

Adds plugin that allows marking IMAP messages as read

f63193a

Implementating IMAP read feature directly in the daemon

50d04b3

pulb approved these changes Nov 1, 2020

View reviewed changes

pulb merged commit b59ef66 into pulb:master Nov 1, 2020

andia89 deleted the markasread branch November 3, 2020 07:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds plugin that allows marking IMAP messages as read #215

Adds plugin that allows marking IMAP messages as read #215

andia89 commented Oct 2, 2020

pulb commented Oct 5, 2020

andia89 commented Oct 5, 2020

pulb commented Oct 6, 2020 via email

pulb commented Oct 31, 2020

pulb Oct 31, 2020

andia89 Nov 3, 2020

pulb Oct 31, 2020

andia89 Nov 3, 2020

pulb Oct 31, 2020

andia89 Nov 3, 2020

pulb Oct 31, 2020

andia89 Nov 3, 2020

pulb Oct 31, 2020

pulb Oct 31, 2020

pulb Oct 31, 2020

pulb commented Nov 1, 2020

pulb commented Nov 3, 2020 via email

pulb commented Nov 4, 2020

andia89 commented Nov 6, 2020

pulb commented Nov 6, 2020 via email

andia89 commented Feb 5, 2021

pulb commented Feb 5, 2021 via email

pulb commented Feb 5, 2021 via email

Adds plugin that allows marking IMAP messages as read #215

Adds plugin that allows marking IMAP messages as read #215

Conversation

andia89 commented Oct 2, 2020

pulb commented Oct 5, 2020

andia89 commented Oct 5, 2020

pulb commented Oct 6, 2020 via email

pulb commented Oct 31, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pulb commented Nov 1, 2020

pulb commented Nov 3, 2020 via email

pulb commented Nov 4, 2020

andia89 commented Nov 6, 2020

pulb commented Nov 6, 2020 via email

andia89 commented Feb 5, 2021

pulb commented Feb 5, 2021 via email

pulb commented Feb 5, 2021 via email