allow selection of contenttype by ninewise · Pull Request #1183 · pazz/alot

ninewise · 2017-12-16T10:28:34Z

Fixes #405.

Issues:

Alot crashes when I swap to a content tab with less content, because we try to select the same line in every content tab (the same line might not exist).
A message with multipart/mixed, but only a single part (the attachment), fails to open (example for self: thread:0000000000006739)
Body of neither text nor html is shown (example for self: thread:0000000000007fc1)

ninewise · 2018-01-02T13:48:02Z

I fixed the failing tests.

The 2 codeclimate issues are actually issues already there before I started modifying these functions, should I try to fix them? (What is cognitive complexity anyway?)

I'm not sure what's wrong with the first travis job, it doesn't show any errors but still fails.

pazz · 2018-01-02T19:19:11Z

alot/widgets/thread.py

            self._sourcetree = TextlinesList(sourcetxt, att, att_focus)
        return self._sourcetree

+    def _get_content_parts(self):


I think this logic should not be part of the FocusableText widget tbh.

It isn't? 😕 It's in MessageTree.

Oh sorry, my bad: Github folding tricked me into believing the change is in FocussableText. I take it back :)

lucc

I did not read the main part of the code. Just some corner cases.

lucc · 2018-01-03T10:47:57Z

alot/commands/thread.py

+    repeatable = True
+
+    def __init__(self, **kwargs):
+        Command.__init__(self, **kwargs)


Do you need this constructor? Doesn't it inherit the parent constructor automatically?

lucc · 2018-01-03T10:51:47Z

alot/db/utils.py

+                 for part
+                 in typed_subpart_iterator(mail, *preferred.split('/'))]
+
+    # should we not, ask for all types


Unclear comment. Is this a question/todo or is it an explanation?

lucc · 2018-01-03T11:36:27Z

I like this feature very much. I tried to implement a different way to navigate the mime tree some time back (#894) but never finished it as I was stuck with some urwidtree issues. So I hope we can merge this feature but here are some more points for consideration/discussion/todo:

Tests for new code would be nice. Alot aims to improve its testsuite so a new feature and new code is always a good chance to add some more tests.
I have prefer_plaintext = True in my config but it is not respected any more. I always get the first mime part initially, no matter its type (I admit it is not very often that you get a mail that has a html part before the plain text part, I only found one in my mail store after a short search)
We might want a :previousctype command for symmetry but that can also wait for a later PR (also do we want to jump to one ctype directly or allow count arguments for these commands?)
It could maybe be named :nextpart as there might be several parts that do not represent the same information in different types but actually different info. (eg. a mailing list digest from the notmuch mailing list)
I am unsure about the visual representation. For me the ctype indication merges with the body but it should feel more like a part of the header information. Or even something like a tabline below the headers. Also mails might have attachements and be signed or encrypted. So we should spend some thought on different cases and how to represent them.

BTW I tried the PR and it works :)

ninewise · 2018-01-03T16:45:57Z

Tests should indeed be written. I'll postpone them because I'm lazy and we might still rename some functions though.
My PR actually brakes/changes the prefer_plaintext option. AFAIK, it now only influences which text is quoted when replying to an email. I'll select the preferred part by default (but not change the order of the parts) and ensure the quoted text is the currently visible one, not the preferred ctype.
A "previous" variant could be trivially added, so I'll add it to here. Only reason I didn't already is, I couldn't think of a good keybinding for it.
:nextpart sounds better. I'll rename this later unless someone disagrees.
Visual representation is hard. I tried something with trees first, but I got stuck with cycles in the MessageTree :-/. I'd appreciate some thoughts on this.

And yeah, it works ;-)

lucc · 2018-01-03T17:47:27Z

Your suggestion in the second point sounds good: select preferred part when opening a mail and use current part when replying. (I have a test mail ready to try it ;)

The cycles in trees are exactly what stopped me in my tracks back in #894.

pazz · 2018-01-04T15:36:42Z

what do you mean by cycles in the tree?

ninewise · 2018-01-05T15:28:34Z

So I had to do something soft of ugly to quote the selected part, but I'm not sure it can be improved without overhauling a lot of command-related-code. Is it even possible to reply to some message that's not the ui.current_buffer.get_selected_message()?

ninewise · 2018-01-05T15:33:13Z

About the cycles, it's probably because I did something wrong with the parent pointers, and at some point while holding the down arrow, all parents are suddenly erased and the parents are gone. Holding the up arrows goes through an infinite cycle of messages.

lucc · 2018-01-06T13:51:01Z

preferring plaintext works again.

lucc · 2018-01-06T13:52:30Z

maybe also something for the todo list: rebase and clean up history with nice logical steps in the commit log and descriptive messages.

ninewise · 2018-12-16T12:03:39Z

Rebased this feature onto the new master.

ninewise · 2018-12-16T19:02:21Z

I need some help to finish this:

Are there some examples of graphical tests anywhere? I can write a test for the quote function, but all other tests are graphical and I'm not seeing any in tests/widgets, yet.
Is there a way to set the position of the cursor? I'd use this to resolve the first issue in the top post here, but I'm not finding how in the urwidtrees documentation.

ninewise · 2020-06-21T07:39:17Z

I believe this was superseded by #1480 so I'm closing this.

ninewise force-pushed the feature/contenttype-selection branch 2 times, most recently from 3bd0793 to 1a13e74 Compare December 18, 2017 09:36

ninewise force-pushed the feature/contenttype-selection branch from 1a13e74 to c7e7816 Compare January 2, 2018 13:37

pazz reviewed Jan 2, 2018

View reviewed changes

lucc requested changes Jan 3, 2018

View reviewed changes

ninewise force-pushed the feature/contenttype-selection branch from 93a986c to 219ca8c Compare January 12, 2018 16:33

Repository owner deleted a comment Feb 15, 2018

ninewise force-pushed the feature/contenttype-selection branch from e1c6ec2 to f740ff7 Compare March 15, 2018 11:38

ninewise force-pushed the feature/contenttype-selection branch 2 times, most recently from 9ea5ee3 to 4afb878 Compare April 3, 2018 13:41

lucc added the feature label Oct 12, 2018

ninewise force-pushed the feature/contenttype-selection branch 2 times, most recently from 3ef23d7 to 44468a7 Compare December 16, 2018 12:01

ninewise force-pushed the feature/contenttype-selection branch from 44468a7 to 57b6966 Compare December 20, 2018 09:17

ninewise force-pushed the feature/contenttype-selection branch from 57b6966 to b2d979e Compare December 20, 2018 10:47

ninewise force-pushed the feature/contenttype-selection branch from b2d979e to 31f7ab9 Compare August 1, 2019 11:51

Felix Van der Jeugt added 4 commits August 5, 2019 16:47

allow selection of multipart part

c40d97e

allow mails without inlined multipart alternatives

c4a062a

show preferred content type if available

a381257

quote selected part on reply/forward

3657c04

ninewise force-pushed the feature/contenttype-selection branch from 31f7ab9 to 3657c04 Compare August 5, 2019 14:48

ninewise closed this Jun 21, 2020

Conversation

ninewise commented Dec 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ninewise commented Jan 2, 2018

Uh oh!

pazz Jan 2, 2018

Choose a reason for hiding this comment

Uh oh!

ninewise Jan 2, 2018

Choose a reason for hiding this comment

Uh oh!

pazz Jan 3, 2018

Choose a reason for hiding this comment

Uh oh!

lucc left a comment

Choose a reason for hiding this comment

Uh oh!

lucc Jan 3, 2018

Choose a reason for hiding this comment

Uh oh!

lucc Jan 3, 2018

Choose a reason for hiding this comment

Uh oh!

lucc commented Jan 3, 2018

Uh oh!

ninewise commented Jan 3, 2018

Uh oh!

lucc commented Jan 3, 2018

Uh oh!

pazz commented Jan 4, 2018

Uh oh!

ninewise commented Jan 5, 2018

Uh oh!

ninewise commented Jan 5, 2018

Uh oh!

lucc commented Jan 6, 2018

Uh oh!

lucc commented Jan 6, 2018

Uh oh!

ninewise commented Dec 16, 2018

Uh oh!

ninewise commented Dec 16, 2018

Uh oh!

ninewise commented Jun 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ninewise commented Dec 16, 2017 •

edited

Loading