Handling of messages requires more thought. #3

Closed
skx opened this Issue May 11, 2013 · 2 comments

1 participant

@skx

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() )

etc.

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

 check_for_mail()

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.)

@skx

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...

@skx skx was assigned May 11, 2013
@skx skx added a commit that referenced this issue May 13, 2013
@skx skx 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.
e6ad44b
@skx skx added a commit that referenced this issue May 13, 2013
@skx skx 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.
2c154cc
@skx

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.

@skx skx added a commit that closed this issue May 15, 2013
@skx skx Messages are now persistant.
  This solves the problem of reading a message causing it to
 become removed from the display.

  Closes #3.
27141bb
@skx skx closed this in 27141bb May 15, 2013
@skx skx removed their assignment Oct 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment