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

Add support for thread folding in mu4e-headers #783

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
9 participants
@zakkak
Contributor

zakkak commented Feb 1, 2016

Implementation for rfe #75
Add ability to fold/unfold threads.

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Feb 1, 2016

Contributor

I use it with

(define-key 'mu4e-headers-mode-map (kbd "TAB") 'mu4e-headers-toggle-thread-folding)

TAB folds the current thread (even if point is not in the root message).
C-u TAB folds the subthread, i.e., all messages bellow the message at point.
TAB on a folded thread/subthread unfolds it.

Works with nested foldings as well, e.g., folding a subthread and then the whole thread.

Contributor

zakkak commented Feb 1, 2016

I use it with

(define-key 'mu4e-headers-mode-map (kbd "TAB") 'mu4e-headers-toggle-thread-folding)

TAB folds the current thread (even if point is not in the root message).
C-u TAB folds the subthread, i.e., all messages bellow the message at point.
TAB on a folded thread/subthread unfolds it.

Works with nested foldings as well, e.g., folding a subthread and then the whole thread.

@djcb

This comment has been minimized.

Show comment
Hide comment
@djcb

djcb Feb 1, 2016

Owner

Oh, nice work! Some minor issue - seems tabbing takes the point to the next line; would be nice if type tab twice would return to the starting situation.

Another thing -- would it be hard to allow for buffer-wide operation, ie. collapse / uncollapse all threads? Perhaps with mu4e-headers-for-each?

Owner

djcb commented Feb 1, 2016

Oh, nice work! Some minor issue - seems tabbing takes the point to the next line; would be nice if type tab twice would return to the starting situation.

Another thing -- would it be hard to allow for buffer-wide operation, ie. collapse / uncollapse all threads? Perhaps with mu4e-headers-for-each?

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Feb 1, 2016

Contributor

seems tabbing takes the point to the next line

Yes, that was on purpose. I was thinking that since one wants to fold a thread probably he is not interested in it and one would like to move to the next thread. I was inspired by the way we mark threads.

would be nice if type tab twice would return to the starting situation.

That makes sense as well.

Another thing -- would it be hard to allow for buffer-wide operation, ie. collapse / uncollapse all threads? Perhaps with mu4e-headers-for-each?

If it is guaranteed that all messages belonging to the same thread are traversed one after another with mu4e-headers-for-each, then it should be easy.

Actually, this PR should be the start for more advanced features. For instance:

  • Start view with all threads folded
  • Add the number of unread messages in the string replacing the overlay (currently set to "...").
  • Mark folded threads with the face (and maybe flags) of any unread messages. This way one can see which folded threads have unread messages.
  • "Arbitrary folding along the lines of gmail where previously read messages in a thread are folded automatically" as per @seanfarley.

The last two seem a long shot, but if you want I could implement the first two in this PR, or create new ones after this one gets merged upstream.

NOTE: I have found a small issue. If you press the down arrow instead of 'n' to get to a message that is not threaded (has no replies) and press tab then the cursor moves to the end of the previous line, but the correct line stays highlighted. I don't know why this happens, if you have any clue, please enlighten me :)

Contributor

zakkak commented Feb 1, 2016

seems tabbing takes the point to the next line

Yes, that was on purpose. I was thinking that since one wants to fold a thread probably he is not interested in it and one would like to move to the next thread. I was inspired by the way we mark threads.

would be nice if type tab twice would return to the starting situation.

That makes sense as well.

Another thing -- would it be hard to allow for buffer-wide operation, ie. collapse / uncollapse all threads? Perhaps with mu4e-headers-for-each?

If it is guaranteed that all messages belonging to the same thread are traversed one after another with mu4e-headers-for-each, then it should be easy.

Actually, this PR should be the start for more advanced features. For instance:

  • Start view with all threads folded
  • Add the number of unread messages in the string replacing the overlay (currently set to "...").
  • Mark folded threads with the face (and maybe flags) of any unread messages. This way one can see which folded threads have unread messages.
  • "Arbitrary folding along the lines of gmail where previously read messages in a thread are folded automatically" as per @seanfarley.

The last two seem a long shot, but if you want I could implement the first two in this PR, or create new ones after this one gets merged upstream.

NOTE: I have found a small issue. If you press the down arrow instead of 'n' to get to a message that is not threaded (has no replies) and press tab then the cursor moves to the end of the previous line, but the correct line stays highlighted. I don't know why this happens, if you have any clue, please enlighten me :)

@djcb

This comment has been minimized.

Show comment
Hide comment
@djcb

djcb Feb 1, 2016

Owner

Cool!

For the first item -- I think I'd prefer to get us to back
to starting position, just like e.g. org-mode does. We could have a
different function (perhaps bound to ) do the tab-and-next,
which can be useful as well. What do you think?

For the other items - indeed, mu4e-headers-for-each works in
headers-order, so that should work fine. Perhaps easiest for now would
be to do any collapsing into some kind of hook after all headers are
listed.

And agreed, let's look at the more complicated ones after the simpler ones are
there.

Anyway, with the tab-change, I think we can take the PR, and then work on
the rest. Nice things!

Owner

djcb commented Feb 1, 2016

Cool!

For the first item -- I think I'd prefer to get us to back
to starting position, just like e.g. org-mode does. We could have a
different function (perhaps bound to ) do the tab-and-next,
which can be useful as well. What do you think?

For the other items - indeed, mu4e-headers-for-each works in
headers-order, so that should work fine. Perhaps easiest for now would
be to do any collapsing into some kind of hook after all headers are
listed.

And agreed, let's look at the more complicated ones after the simpler ones are
there.

Anyway, with the tab-change, I think we can take the PR, and then work on
the rest. Nice things!

@djcb

This comment has been minimized.

Show comment
Hide comment
@djcb

djcb Feb 1, 2016

Owner

Oh, and I'll take a look at that small issue mentioned.

Owner

djcb commented Feb 1, 2016

Oh, and I'll take a look at that small issue mentioned.

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Feb 1, 2016

Contributor

The issue seems to disappear when I use save-excursion, which I will from now on. As a result you can ignore it, it won't exist in the next push ;)

Contributor

zakkak commented Feb 1, 2016

The issue seems to disappear when I use save-excursion, which I will from now on. As a result you can ignore it, it won't exist in the next push ;)

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Feb 2, 2016

Contributor

The function now takes two arguments, one for subthread folding and another for moving to the next thread after folding.

I also implemented the message counting. When folded, it will print +N where N is the number of messages in the folded thread/subthread. It also counts unread messages and displays them next to the total count like +N(M).

Ignore my previous comment about the issue, it looks like I can't always rely on save-excursion. The problem is with folding a thread while in it (not at its first message). In this case I have to move the cursor to the first message in the thread. So the glitch is still here, it is weird though that it only appears when moving down with the arrow key and then folding while on the first message of a thread.

I also had to play around with the position of the cursor for the overlays. Leaving it to the end-of-line results in the point appearing next to the display string, so I move it by -1. The above misbehavior might also relate to this.

Another bug/feature I observe is that when folding nested folds the display results in something like +7(3) +4(1) +2(1) where each +N(M) is the number of messages (and unread messages) in each folding level, from outermost to innermost.

Contributor

zakkak commented Feb 2, 2016

The function now takes two arguments, one for subthread folding and another for moving to the next thread after folding.

I also implemented the message counting. When folded, it will print +N where N is the number of messages in the folded thread/subthread. It also counts unread messages and displays them next to the total count like +N(M).

Ignore my previous comment about the issue, it looks like I can't always rely on save-excursion. The problem is with folding a thread while in it (not at its first message). In this case I have to move the cursor to the first message in the thread. So the glitch is still here, it is weird though that it only appears when moving down with the arrow key and then folding while on the first message of a thread.

I also had to play around with the position of the cursor for the overlays. Leaving it to the end-of-line results in the point appearing next to the display string, so I move it by -1. The above misbehavior might also relate to this.

Another bug/feature I observe is that when folding nested folds the display results in something like +7(3) +4(1) +2(1) where each +N(M) is the number of messages (and unread messages) in each folding level, from outermost to innermost.

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Feb 2, 2016

Contributor

We should probably make the display string customizable at some point.

Contributor

zakkak commented Feb 2, 2016

We should probably make the display string customizable at some point.

@djcb

This comment has been minimized.

Show comment
Hide comment
@djcb

djcb Feb 3, 2016

Owner

Cool stuff, @zakkak! If you can fix those little things (hopefully easy to fix!), I'll merge this. I think it would be quite an improvement for mu4e!

Owner

djcb commented Feb 3, 2016

Cool stuff, @zakkak! If you can fix those little things (hopefully easy to fix!), I'll merge this. I think it would be quite an improvement for mu4e!

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Feb 3, 2016

Contributor

I think I will need some help to fix them. I am not very familiar with moving the point around.

Does mu4e introduce any invisible stuff at the beginning or the end of each line in the mu4e-headers view?

Contributor

zakkak commented Feb 3, 2016

I think I will need some help to fix them. I am not very familiar with moving the point around.

Does mu4e introduce any invisible stuff at the beginning or the end of each line in the mu4e-headers view?

@djcb

This comment has been minimized.

Show comment
Hide comment
@djcb

djcb Feb 4, 2016

Owner

Okay, I'll play around with it. And yes, there's a marker at the beginning of lines; see mu4e~headers-docid-pre

Owner

djcb commented Feb 4, 2016

Okay, I'll play around with it. And yes, there's a marker at the beginning of lines; see mu4e~headers-docid-pre

@djcb

This comment has been minimized.

Show comment
Hide comment
@djcb

djcb Feb 5, 2016

Owner

I've pushed a zakkak/thread-folding branch here. Let's see if I have some hacking time this weekend...

As for #' vs ' - they are not the same, but I don't the difference matters much. But let's use the latter, it's simpler.

Owner

djcb commented Feb 5, 2016

I've pushed a zakkak/thread-folding branch here. Let's see if I have some hacking time this weekend...

As for #' vs ' - they are not the same, but I don't the difference matters much. But let's use the latter, it's simpler.

@andersjohansson

This comment has been minimized.

Show comment
Hide comment
@andersjohansson

andersjohansson Feb 16, 2016

Contributor

Regarding #' and ':

Use a sharp quote (#') when quoting function names. It's a good hint for the byte-compiler, which will warn you if the function is undefined. Some macros can also behave differently otherwise (like cl-labels).

https://github.com/bbatsov/emacs-lisp-style-guide

Don't ask me about what difference it really makes though...

Contributor

andersjohansson commented Feb 16, 2016

Regarding #' and ':

Use a sharp quote (#') when quoting function names. It's a good hint for the byte-compiler, which will warn you if the function is undefined. Some macros can also behave differently otherwise (like cl-labels).

https://github.com/bbatsov/emacs-lisp-style-guide

Don't ask me about what difference it really makes though...

@hexa00

This comment has been minimized.

Show comment
Hide comment
@hexa00

hexa00 Aug 12, 2016

Hi,
I've been using this branch for a few months and really like it :)

However I really wish I could fold/unfold all threads, has anyone figured this out already ?

I tried the naive way calling like :

(defun mu4e-headers-fold-all ()
"Folds all threads"
(interactive)
(mu4e-headers-for-each 'mu4e-headers-toggle-thread-folding))

But that was too naive it seems...

Thanks,

hexa00 commented Aug 12, 2016

Hi,
I've been using this branch for a few months and really like it :)

However I really wish I could fold/unfold all threads, has anyone figured this out already ?

I tried the naive way calling like :

(defun mu4e-headers-fold-all ()
"Folds all threads"
(interactive)
(mu4e-headers-for-each 'mu4e-headers-toggle-thread-folding))

But that was too naive it seems...

Thanks,

@dakrone

This comment has been minimized.

Show comment
Hide comment
@dakrone

dakrone Sep 27, 2016

What's the word on this, is it something that's likely to be merged into mu4e, or something users should use a custom build to add support for?

dakrone commented Sep 27, 2016

What's the word on this, is it something that's likely to be merged into mu4e, or something users should use a custom build to add support for?

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Sep 27, 2016

Contributor

It is still far from perfect. Since I am pretty busy the last few months I don't know when and if I will manage to get it in a good shape to be merged upstream.

Any contributions are welcome.

Contributor

zakkak commented Sep 27, 2016

It is still far from perfect. Since I am pretty busy the last few months I don't know when and if I will manage to get it in a good shape to be merged upstream.

Any contributions are welcome.

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Dec 30, 2016

Contributor

Bump. I finally got some time to take a look at the code and fix a few things.

So, folding seems to work better now.

Known issues

Marking folded threads:

  1. applies mark only to visible message (maybe this is OK as is) and
  2. breaks visualization by hiding the message and showing only the overlay display (not OK).
Contributor

zakkak commented Dec 30, 2016

Bump. I finally got some time to take a look at the code and fix a few things.

So, folding seems to work better now.

Known issues

Marking folded threads:

  1. applies mark only to visible message (maybe this is OK as is) and
  2. breaks visualization by hiding the message and showing only the overlay display (not OK).
@hexa00

This comment has been minimized.

Show comment
Hide comment
@hexa00

hexa00 Jan 9, 2017

Hi,
Thanks for working on this !

I just updated and I notice this small issue when folding I get:
Subject +8(8)h

Notice the 'h" , this can be any char, most likely tied to some text in the area....

Also, not sure I understand known issue #2.. ?

hexa00 commented Jan 9, 2017

Hi,
Thanks for working on this !

I just updated and I notice this small issue when folding I get:
Subject +8(8)h

Notice the 'h" , this can be any char, most likely tied to some text in the area....

Also, not sure I understand known issue #2.. ?

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Jan 9, 2017

Contributor

Hi,

I can't reproduce the Subject +8(8)h issue.

Regarding the second Known Issue, it refers to applying some mark at a folded thread. After applying the mark the whole line disappears for me and I am only left with the counts, e.g., Date | From | Subject +8(8) becomes +8(8).

Contributor

zakkak commented Jan 9, 2017

Hi,

I can't reproduce the Subject +8(8)h issue.

Regarding the second Known Issue, it refers to applying some mark at a folded thread. After applying the mark the whole line disappears for me and I am only left with the counts, e.g., Date | From | Subject +8(8) becomes +8(8).

@hexa00

This comment has been minimized.

Show comment
Hide comment
@hexa00

hexa00 Jan 9, 2017

You mean like doing on a folded thread ?

That works fine here, only issue is that the cursor goes to the end of the line...

hexa00 commented Jan 9, 2017

You mean like doing on a folded thread ?

That works fine here, only issue is that the cursor goes to the end of the line...

@hexa00

This comment has been minimized.

Show comment
Hide comment
@hexa00

hexa00 Jan 9, 2017

Oops I mean doing "insert" on a folded thread,, I used '<' and '>' and github just removed the tag...

hexa00 commented Jan 9, 2017

Oops I mean doing "insert" on a folded thread,, I used '<' and '>' and github just removed the tag...

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Jan 9, 2017

Contributor

Hmmm, <insert> seems to not trigger the issue, try marking a folded thread as read and apply the action.

Contributor

zakkak commented Jan 9, 2017

Hmmm, <insert> seems to not trigger the issue, try marking a folded thread as read and apply the action.

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Jan 9, 2017

Contributor

More accurately if you choose to apply a mark after <insert> the problem appears again.

Contributor

zakkak commented Jan 9, 2017

More accurately if you choose to apply a mark after <insert> the problem appears again.

@hexa00

This comment has been minimized.

Show comment
Hide comment
@hexa00

hexa00 Jan 9, 2017

OK I see, I can reproduce now, thanks for the explaination. (Note that I get +8(8)h )

On another note the only big missing features for me would be to:

  • Have some way to fold all threads.
  • Keep the folded thread folded when mu refreshes...

Have you considered those ?

Thanks,

hexa00 commented Jan 9, 2017

OK I see, I can reproduce now, thanks for the explaination. (Note that I get +8(8)h )

On another note the only big missing features for me would be to:

  • Have some way to fold all threads.
  • Keep the folded thread folded when mu refreshes...

Have you considered those ?

Thanks,

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Jan 9, 2017

Contributor

Well those make sense and would be great to have.
One step at a time :)

Contributor

zakkak commented Jan 9, 2017

Well those make sense and would be great to have.
One step at a time :)

@hexa00

This comment has been minimized.

Show comment
Hide comment
@hexa00

hexa00 Jan 9, 2017

hehe OK :) Just making sure I haven't missed it.

Thanks again!

hexa00 commented Jan 9, 2017

hehe OK :) Just making sure I haven't missed it.

Thanks again!

@zakkak zakkak referenced this pull request Jan 25, 2017

Open

add thread fold/unfold #75

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Jan 25, 2017

Contributor

@djcb could you please have a look in to this and let me know of any comments/directions?

Contributor

zakkak commented Jan 25, 2017

@djcb could you please have a look in to this and let me know of any comments/directions?

@djcb

This comment has been minimized.

Show comment
Hide comment
@djcb

djcb Jan 25, 2017

Owner

He @zakkak, nice to see this! I like the functionality; the implementation is a bit complicated (probably necessarily so). Anyway, are you happy with how it works? Ie., does it all work as expected?

Owner

djcb commented Jan 25, 2017

He @zakkak, nice to see this! I like the functionality; the implementation is a bit complicated (probably necessarily so). Anyway, are you happy with how it works? Ie., does it all work as expected?

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Jan 26, 2017

Contributor

Other than not supporting marking folded threads (#783 (comment)), I am pretty happy about it and use it on a daily base.

Contributor

zakkak commented Jan 26, 2017

Other than not supporting marking folded threads (#783 (comment)), I am pretty happy about it and use it on a daily base.

@felipeochoa

This comment has been minimized.

Show comment
Hide comment
@felipeochoa

felipeochoa Mar 16, 2017

I've refactored this a bit to split out the complexity into different functions[1]. I've also added a mu4e-headers-fold-all that I like adding to mu4e-headers-found-hook. There's also now an option to override the "folded slug" formatting under mu4e-headers-folding-slug-function.

There are a couple of small bugs in the folding:

  1. Pressing n on a header with a folded thread moves point to EOL instead of down to the next header. I think the fix is in mu4e~headers-move, where forward-line needs to become forward-visible-line, but I'm not sure.
  2. Marking a header while point is on the overlay breaks the folding. There are two solutions here:
    • The quick one: This happens because mu4e-mark-at-point calls an unqualified remove-overlays, splitting the folding overlay. If instead these overlays defined some property like mu4e-marking-overlay, the call to remove-overlays could filter these out and avoid this problem.
    • The complete one: We should probably keep a hash of folded messages. That way we can forbid marking of folded headers with a check in mu4e-mark-at-point or mu4e~headers-mark. This hash could also be used re-instate folding after an index update

Sorry I don't have time to make this into a PR and fix these bugs, but hopefully this pushes the ball forward a bit.

[1] https://gist.github.com/felipeochoa/614308ac9d2c671a5830eb7847985202

felipeochoa commented Mar 16, 2017

I've refactored this a bit to split out the complexity into different functions[1]. I've also added a mu4e-headers-fold-all that I like adding to mu4e-headers-found-hook. There's also now an option to override the "folded slug" formatting under mu4e-headers-folding-slug-function.

There are a couple of small bugs in the folding:

  1. Pressing n on a header with a folded thread moves point to EOL instead of down to the next header. I think the fix is in mu4e~headers-move, where forward-line needs to become forward-visible-line, but I'm not sure.
  2. Marking a header while point is on the overlay breaks the folding. There are two solutions here:
    • The quick one: This happens because mu4e-mark-at-point calls an unqualified remove-overlays, splitting the folding overlay. If instead these overlays defined some property like mu4e-marking-overlay, the call to remove-overlays could filter these out and avoid this problem.
    • The complete one: We should probably keep a hash of folded messages. That way we can forbid marking of folded headers with a check in mu4e-mark-at-point or mu4e~headers-mark. This hash could also be used re-instate folding after an index update

Sorry I don't have time to make this into a PR and fix these bugs, but hopefully this pushes the ball forward a bit.

[1] https://gist.github.com/felipeochoa/614308ac9d2c671a5830eb7847985202

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Mar 16, 2017

Contributor

It sure does. Thanks for your contributions.

I will try to get your changes on my branch, but it would be better if you could create a PR.
In the meantime if anyone else is willing to do the PR, it will be much appreciated.

Contributor

zakkak commented Mar 16, 2017

It sure does. Thanks for your contributions.

I will try to get your changes on my branch, but it would be better if you could create a PR.
In the meantime if anyone else is willing to do the PR, it will be much appreciated.

Add support for thread folding in mu4e-headers
Implementation for rfe #75
Add ability to fold/unfold threads.
@sebhahn

This comment has been minimized.

Show comment
Hide comment
@sebhahn

sebhahn Aug 7, 2017

Is this feature merged anytime soon?

sebhahn commented Aug 7, 2017

Is this feature merged anytime soon?

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Sep 6, 2017

Contributor

Probably not, it is not stable yet.

Unfortunately I don't have much time to work on it so if anyone is willing to take it over, it would be great.

Contributor

zakkak commented Sep 6, 2017

Probably not, it is not stable yet.

Unfortunately I don't have much time to work on it so if anyone is willing to take it over, it would be great.

@stefanv

This comment has been minimized.

Show comment
Hide comment
@stefanv

stefanv Sep 7, 2017

@zakkak Can you expand a bit on what needs to be done?

stefanv commented Sep 7, 2017

@zakkak Can you expand a bit on what needs to be done?

@zakkak

This comment has been minimized.

Show comment
Hide comment
@zakkak

zakkak Sep 26, 2017

Contributor

There are the issues mentioned here regarding marking and applying actions on folded threads.
There is also this patch that needs to be merged and it also lists some more issues.

Contributor

zakkak commented Sep 26, 2017

There are the issues mentioned here regarding marking and applying actions on folded threads.
There is also this patch that needs to be merged and it also lists some more issues.

@titaniumbones

This comment has been minimized.

Show comment
Hide comment
@titaniumbones

titaniumbones Oct 1, 2018

Contributor

I'm not in a position to track down and fix these bugs but just want to add a word of encouragement to whomever might be able to od it -- I've just loaded @felipeochoa 's gist version and while I'm sure at some point the bugs that @zakkak mentions will become frustrating, already I notice a dramatic improvement in usage of mu4e. I would love to see this merged to master so everyone can take advantage of it.

Contributor

titaniumbones commented Oct 1, 2018

I'm not in a position to track down and fix these bugs but just want to add a word of encouragement to whomever might be able to od it -- I've just loaded @felipeochoa 's gist version and while I'm sure at some point the bugs that @zakkak mentions will become frustrating, already I notice a dramatic improvement in usage of mu4e. I would love to see this merged to master so everyone can take advantage of it.

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