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

Don't lose the focus when refreshing a thread #1499

Merged
merged 1 commit into from
May 10, 2020

Conversation

lenormf
Copy link
Contributor

@lenormf lenormf commented May 1, 2020

This commit allows keeping the currently focused message selected
across thread refreshes.

Since this operation can take a long time in large threads, a
search_threads_rebuild_limit option is introduced. When defined,
it sets the maximum amount of messages that will be iterated upon
when trying to find the previously focused message.

Copy link
Owner

@pazz pazz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously a hack but it patches over some intrinsic urwid/notmuch problems that lots of users seem to care about, so I'm willing ot add it.

Please fix the avoidable codeclimate issues where possible.
I've triggered travis to re-run the docs generation. Something fails there. This needs to be fixed before merging.

@pazz pazz mentioned this pull request May 6, 2020
@lenormf lenormf force-pushed the keep_focus_refresh branch from 084d707 to 08a9bf5 Compare May 6, 2020 10:30
@lenormf
Copy link
Contributor Author

lenormf commented May 6, 2020

I fixed the codeclimate issue I could, it looks like some Travis builds fail because of documentation problems? Have I edited the wrong files?

@pazz
Copy link
Owner

pazz commented May 6, 2020

Thanks. Did you rebase to master already? I believe these travis-docs issues are fixed on master..

@lenormf lenormf force-pushed the keep_focus_refresh branch from 08a9bf5 to 0b4284b Compare May 6, 2020 11:03
@lenormf
Copy link
Contributor Author

lenormf commented May 6, 2020

I rebased on b11aa98, still the same build errors :/

@pazz
Copy link
Owner

pazz commented May 6, 2020

Ah right: you've changed some config options but did not re-generate the docs locally (which will generate some tables that need to be checked in).

make -C docs cleanall html

I saw that you did this for your original R but perhaps not after the rebase..
Also, could you try and avoid the lines-too-long error if possible?

alot/buffers/search.py Outdated Show resolved Hide resolved
This commit allows keeping the currently focused message selected
across thread refreshes.

Since this operation can take a long time in large threads, a
`search_threads_rebuild_limit` option is introduced. When defined,
it sets the maximum amount of messages that will be iterated upon
when trying to find the previously focused message.
@lenormf lenormf force-pushed the keep_focus_refresh branch from 0b4284b to a51c3ea Compare May 6, 2020 14:09
@pazz
Copy link
Owner

pazz commented May 6, 2020

This looks good to me now, but with out having tested this.
I'd appreciate some positive user test before I merge this...

@pazz
Copy link
Owner

pazz commented May 8, 2020

I've tested this myself just now and it looks quite good.
One problem, which we need not necessarily fix here, is that there is no perceivable user feedback upon a refresh in search mode.

It should be nice do include a similar patch for thread mode as well, where I personally use the refresh command much more often to check on new mails in a thread.

It seems to have exposed a (unrelated?) bug in the email module for me:
search for * and scroll down a few pages, then hit @ for refresh.
The logs show this trace (and I get a corresponding popup) occasionally:

 File "/home/pazz/projects/alot/alot/ui.py", line 723, in apply_command
    cmd.apply(self)
  File "/home/pazz/projects/alot/alot/commands/globals.py", line 172, in apply
    ui.update()
  File "/home/pazz/projects/alot/alot/ui.py", line 668, in update
    self.mainloop.draw_screen()
  File "/usr/lib/python3/dist-packages/urwid/main_loop.py", line 586, in draw_screen
    canvas = self._topmost_widget.render(self.screen_size, focus=True)
  File "/usr/lib/python3/dist-packages/urwid/widget.py", line 144, in cached_render
    canv = fn(self, size, focus=focus)
  File "/usr/lib/python3/dist-packages/urwid/decoration.py", line 226, in render
    canv = self._original_widget.render(size, focus=focus)
  File "/usr/lib/python3/dist-packages/urwid/widget.py", line 144, in cached_render
    canv = fn(self, size, focus=focus)
  File "/usr/lib/python3/dist-packages/urwid/container.py", line 1085, in render
    body = self.body.render((maxcol, maxrow-ftrim-htrim),
  File "/home/pazz/projects/alot/alot/buffers/buffer.py", line 19, in render
    return self.body.render(size, focus)
  File "/usr/lib/python3/dist-packages/urwid/widget.py", line 144, in cached_render
    canv = fn(self, size, focus=focus)
  File "/usr/lib/python3/dist-packages/urwid/listbox.py", line 470, in render
    middle, top, bottom = self.calculate_visible(
  File "/usr/lib/python3/dist-packages/urwid/listbox.py", line 416, in calculate_visible
    next, pos = self._body.get_next( pos )
  File "/home/pazz/projects/alot/alot/walker.py", line 46, in get_next
    return self._get_at_pos(start_from + self.direction)
  File "/home/pazz/projects/alot/alot/walker.py", line 72, in _get_at_pos
    widget = self._get_next_item()
  File "/home/pazz/projects/alot/alot/walker.py", line 85, in _get_next_item
    next_widget = self.containerclass(next_obj, **self.kwargs)
  File "/home/pazz/projects/alot/alot/widgets/search.py", line 26, in __init__
    self.rebuild()
  File "/home/pazz/projects/alot/alot/widgets/search.py", line 60, in rebuild
    width, part = build_text_part(partname, self.thread,
  File "/home/pazz/projects/alot/alot/widgets/search.py", line 145, in build_text_part
    content = prepare_string(name, thread, maxw)
  File "/home/pazz/projects/alot/alot/widgets/search.py", line 213, in prepare_string
    s = content(thread)
  File "/home/pazz/projects/alot/alot/widgets/search.py", line 178, in prepare_authors_string
    return thread.get_authors_string() or '(None)'
  File "/home/pazz/projects/alot/alot/db/thread.py", line 195, in get_authors_string
    for aname, aaddress in self.get_authors():
  File "/home/pazz/projects/alot/alot/db/thread.py", line 158, in get_authors
    msgs = sorted(self.get_messages().keys(),
  File "/home/pazz/projects/alot/alot/db/thread.py", line 247, in get_messages
    self._toplevel_messages.append(accumulate(self._messages, m))
  File "/home/pazz/projects/alot/alot/db/thread.py", line 237, in accumulate
    M = Message(self._dbman, msg, thread=self)
  File "/home/pazz/projects/alot/alot/db/message.py", line 71, in __init__
    self.mime_part = get_body_part(self.get_email())
  File "/home/pazz/projects/alot/alot/db/utils.py", line 491, in get_body_part
    body_part = mail.get_body(preferencelist)
  File "/usr/lib/python3.8/email/message.py", line 1018, in get_body
    for prio, part in self._find_body(self, preferencelist):
  File "/usr/lib/python3.8/email/message.py", line 989, in _find_body
    yield from self._find_body(subpart, preferencelist)
  File "/usr/lib/python3.8/email/message.py", line 989, in _find_body
    yield from self._find_body(subpart, preferencelist)
  File "/usr/lib/python3.8/email/message.py", line 978, in _find_body
    if part.is_attachment():
AttributeError: 'str' object has no attribute 'is_attachment'

@pazz
Copy link
Owner

pazz commented May 10, 2020

It seems my last issue is unrelated. I have created a new issue for his (#1506).

@pazz pazz merged commit 8e96d50 into pazz:master May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants