New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling of messages requires more thought. #3

skx opened this Issue May 11, 2013 · 2 comments


None yet
1 participant
Copy link

skx commented May 11, 2013

I've backed myself into a corner by having CGlobal::get_messages() honour the index_limit().

If I allow the current message to be marked as read it will immediately disappear from the display post-read.

The alternative is to have get_message retrieve all messages from the selected folders, then have the display function iterate and decide which ones should be displayed.

The problem with that approach is that the same logic must be applied in all cases where the current message is required. For example I expect we'll expose the "current_message()" function to Lua, and that will be used as:

 set_read( current_message() )
 reply( current_message() )
 delete( current_message() )


That means duplicating that "Convert all messages to current subset" logic in multiple places.

Also although we can have the union of all message stored in:

   std::vector<CMessage *>  all_messages

This could be updated when a selected folder is added/removed/cleared, but that won't
cover the case when new mail appears asynchronously. Unless we add an explicit


function. Which I thought we didn't need to do when teh display updated "magically".

I expect this means I need to pause and rethink things.

(Having std::vector< CMessage *> is better than std::vector<CMessage> though, as it allows lazy parsing of the message header/body via mimetic.)


This comment has been minimized.

Copy link
Contributor Author

skx commented May 11, 2013

Having spent most of the day thinking about this I'm choosing to break down the handling of messages in folder(s) into two parts:

  1. Find all messages.
  2. Filter all messages.

The first part is obviously to fetch all possible messages from the selected folders. The second part is to limit the messages such that the appropriate subset may be displayed.

The difference between the current behaviour is that the display code will display both "the messages that match" and "the current message". The notion of a current message will be changed from an integer offset against the list of all messages into a CMessage object - or more likely the path the message contains.

This handles the fetch vs. display case of:

  • fetch all messages.
  • show only new message.
  • open a message, mark it as read. Because it is the "current" message it will still be displayed.

The secondary concern of updating in real-time such that new (incoming) mail is displayed "instantly" is a minor complication, but we can cache based on the mtime of the parent folders, or even fetch each on demand...

@ghost ghost assigned skx May 11, 2013

skx added a commit that referenced this issue May 13, 2013

Updated such that messages are heap-allocated.
  NOTE: This makes a big memory leak each time the subset of
 messages changes.

  This is a transition as I rework the code as per issue #3.

skx added a commit that referenced this issue May 13, 2013

Updated the way that messges are handled.
  Read messages are now marked as such, which causes a bug, as
 expected.  Issue #3 is real and will be a pain to fix.

This comment has been minimized.

Copy link
Contributor Author

skx commented May 14, 2013

As per the last comment showing both things will work:

  • The messages that are associated with the current limit.
  • The current message.

The downside here is that we currently handle navigation by merely incrementing/decrementing the global "offset form start of messages". If we show only new messages then this won't work. Consider the case when there are several messages:

  • new
  • read
  • read
  • new
  • read
  • red
  • red
  • new

If you're showing only "new" messages, and the current message is the first then incrementing the offset by one will jump the the second message, which will suddenly be displayed because it matches the "current message".

Instead we need to rework the next/prev message navigation to instead be "next/prev matching the limit in place". This is doable, but it starts to feel like a mess.

I think this is probably what I'll go for, but I'm not committing anything until I've mulled it over some more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment