On 07/26/11 02:45, Thomas Gahr wrote: > I think I'm onto something here... Sorry for not supplying a patch this time > but I swamped the source with some homebrew trace macros to be able to follow > the execution path a little better... > > I noticed that TreeItemMsgList::_numberFetchingStatus is set to LOADING in: > > ObtainSynchronizedMailboxTask::syncFlags > > but then it is not set to DONE although a reply for fetching flags is > received. > I noticed that ObtainSynchronizedMailboxTask::handleStateHelper is called and > has an if-branch for "resp->tag == flagsCmd" which now looks like this in my > source: (added lines start at //TG)
Hi Thomas, sorry for wasting your time here, I've done something very similar yesterday, but forgot to push :(. Fixed now. Your analysis was spot-on, but I decided to set the _numberFetchingStatus in TreeItemMsgList::recalcVariousMessageCounts() instead. > This fixes the issue for me: when a mailbox is selected and a message is > marked > as read, the mailbox view reacts accordingly. I still have to occasionally generate a mouse over over the mailbox in the left column in order for the GUI to show an updated number; that's likely an issue in my model code. > But1: At first I supposed the method > ObtainSynchronizedMailboxTask::handleFlags > is responsible for handling that response, but it doesn't seem to be called - > at least in my testcase - should the _numberFetchingStatus flag be unset > there > as well? These are different "flags" (it lists all flags available for use in a particular mailbox) than per-message flags, which are handled by ImapTask::handleFetch -> Model::_genericHandleFetch -> TreeItemMailbox::handleFetchResponse -> TreeItemMessage::setFlags. > But2: this manual unsetting of _numberFetchingStatus from several different > places seems cumbersome and error-prone - is it OK for me to have a go and > try > to improve this situation? (accessor method or some RAII-based reference- > counting like magic, I'll have to dig into the source a little further to > think of something specific...) I usually tend not to bother with getters/setters "for simplicity", and then start pulling my hair when debugging problems like this :). I'd say that numberFetchingStatus() and setNumberFetchingStatus() would look great. However, there's a small problem here [1]. I'm still a student, and Trojita is subject of my Master's thesis, which means that there's a limit on contributions I can accept at this point, especially in the src/Imap directory. While the getters/setters are mostly just a cosmetic change, I feel like saying this in advance. Generally speaking, there is no problem with accepting advice (and I'll of course give a proper credit to you in the thesis), small patches, cleanup changes etc, but "big" changes should be limited to be outside of the IMAP features. For example, contributing support for faster IMAP syncing via QRESYNC is currently off limits, while adding support for encrypted/signed e-mails, refactoring the SMTP code or implementing multiple accounts is perfectly acceptable. I hope this won't stop you from playing with Trojita and messing around with its source code; as I said, I'm grateful for every comment I get and would love to improve myself. With kind regards, Jan [1] https://projects.flaska.net/projects/trojita/wiki/Contributing_to_Trojita -- Trojita, a fast e-mail client -- http://trojita.flaska.net/
signature.asc
Description: OpenPGP digital signature
