Skip to content
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

index: fix cursor after new mail #3560

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

index: fix cursor after new mail #3560

wants to merge 2 commits into from

Conversation

flatcap
Copy link
Member

@flatcap flatcap commented Nov 9, 2022

Update: I've fixed some problems, but the issue with <limit> remains.
Unfortunately, there isn't going to be a quick-fix for that.


When new mail arrives, the backends (maildir/imap) update the Mailbox and send notifications.
In the frontend, the Mailbox view (MView) observes these changes (mview_mailbox_observer()).
(MView was previously known as the Context)

Before we do anything that might affect the ordering of the Emails, remember the sequence number of the current Email.
Afterwards, search for the Email by the sequence number.

Fixes: #3544

@flatcap flatcap added the type:bug Bug label Nov 9, 2022
@flatcap flatcap self-assigned this Nov 9, 2022
@flatcap flatcap requested a review from a team as a code owner November 9, 2022 15:11
@igor47
Copy link
Contributor

igor47 commented Nov 29, 2022

didn't work for me for the specific issue i'm seeing:

  1. apply a limit pattern
  2. scroll to a middle message in the limited view
  3. send myself a new email from a different account

i expect that i would remain where it was. instead, the message "New mail in this mailbox" is displayed, and the cursor position is lost/non-existent. pressing j then selects the first message in the mailbox/limited view.

mview.c Fixed Show fixed Hide fixed
@auouymous
Copy link
Contributor

Why do the same unchanged commits keep getting force pushed?

@flatcap
Copy link
Member Author

flatcap commented Jan 28, 2023

Ah! That's me git rebaseing my work branch over main so that I have the latest source.

@auouymous
Copy link
Contributor

The latest push appears to fix the issue for me. I'm installing it on my main client and will let you know if it has any problems. Thank you!

@igor47
Copy link
Contributor

igor47 commented Mar 7, 2023

does not fix it for me. i'm in a limited view, and when i get the notification "new mail in inbox", i still loose my current cursor position. my limited view did not include the new message. my configure (on debian 11.6):

/configure --build=x86_64-linux-gnu --prefix=/usr {--includedir=${prefix}/include} {--mandir=${prefix}/share/man} {--infodir=${prefix}/share/info} --sysconfdir=/etc --localstatedir=/var --disable-silent-rules {--libdir=${prefix}/lib/x86_64-linux-gnu} {--libexecdir=${prefix}/lib/x86_64-linux-gnu} --disable-maintainer-mode --disable-dependency-tracking --mandir=/usr/share/man --libexecdir=/usr/libexec --with-mailpath=/var/mail --gpgme --lua --notmuch --with-ui --gnutls --idn --mixmaster --sasl --tokyocabinet --disable-doc --disable-nls

@flatcap
Copy link
Member Author

flatcap commented Mar 7, 2023

Thanks for the feedback ❤️

I spent the day working on this and made some progress, but I'm not done yet.
The trouble is the Pager combined with limit.

There are eight test cases, combinations of:

  • Index vs Pager
  • Limit vs No limit
  • Inotify enabled vs disabled

@auouymous
Copy link
Contributor

Oh, using a limit breaks both index and pager by setting cursor to an invalid message, and closes pager. This happens for new messages inserted at top or bottom, matching or not matching the limit.

Using inotify.

@auouymous
Copy link
Contributor

New patch fixed the broken cursor after receiving new mail, but pager is still closed when new mail arrives before or after the cursor when a limit is set. It also introduced some new bugs.

Closing pager or changing messages after any mail as arrived does not mark message as old. Switching to another message or opening and closing pager again will mark the message as old.

I think the messages are being marked as old in the UI but not in the backend, and a refresh switches them back to new. So the next two bugs might only happen when messages are new or recently marked as old with no other actions that could sync them to the backend.

Receiving mail while in index or pager causes all old messages to be marked as new, and shows "Mailbox was externally modified".

While in pager, receiving mail before the cursor marks current message as old, updates the index so current message shifts down one, and then marks current message as new.

@auouymous
Copy link
Contributor

Why was this pushed to main in 2523464?

@auouymous
Copy link
Contributor

@flatcap git revert 2523464c1d6b293dc5f512944ed84f96823c6c9d cleanly reverts the patch out of main branch (assuming it was accidentally pushed). The only difference is the double mview.h include that has since been removed.

@flatcap flatcap marked this pull request as draft May 2, 2023 13:02
@flatcap flatcap force-pushed the devel/pager branch 2 times, most recently from 97b0177 to 511fc2e Compare May 15, 2023 17:26
@flatcap flatcap force-pushed the devel/pager branch 3 times, most recently from 89a6426 to 582c545 Compare June 20, 2023 17:07
@flatcap flatcap force-pushed the devel/pager branch 2 times, most recently from 5bb797f to 408923d Compare October 29, 2023 16:23
@flatcap flatcap force-pushed the devel/pager branch 2 times, most recently from 4efe8ea to 64aacee Compare November 16, 2023 19:41
@flatcap flatcap force-pushed the devel/pager branch 3 times, most recently from 2f04c90 to 1445cc4 Compare March 29, 2024 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pager jumps to different message when new messages arrive and pager_index_lines is set
3 participants