alphapapa / ement.el

A Matrix client for GNU Emacs
GNU General Public License v3.0
488 stars 44 forks source link

Make the read receipt marker optional #136

Closed FrostyX closed 1 year ago

FrostyX commented 1 year ago

I have my chat windows on a separate monitor so I can read new messages without moving cursor to the actual window. As a consequence rendering the read receipt markers inaccurate and not useful.

alphapapa commented 1 year ago

Thanks for pointing this out. I understand that, since the room buffers are always visible to you, but the frame isn't always selected, the markers aren't always moved to follow what your eyeballs have actually seen.

But the markers still reflect where the read markers are in the room, which is what the Matrix server considers to be what you've seen. And if we don't update these, that means that other clients (and Ement, next time it's connected) won't know where you left off reading, which can mean, e.g. that another client tries to show you many old messages, or prompts you to go far back in a room's history for no reason.

As well, this patch changes the function's behavior in a fundamental way by making its entire body conditional. This can make a program harder to reason about.

So I don't think this is the best way to solve the issue you're having. Some other ideas I can think of:

  1. Run a non-idle timer that calls ement-room-read-receipt-idle-timer (i.e. it would move the markers regardless of what else Emacs may be doing, whether any of its frames are selected, etc). (However, the timer would probably need to iterate over the frame list and call this function for each of them.)
  2. Simply hide the markers by adjusting their appearance.

What do you think? Thanks.

FrostyX commented 1 year ago

Thank you for the explanation @alphapapa. I also found

(setq ement-room-send-read-receipts nil)

which seems to do the same thing as this PR? So I guess it probably suffers the same issue ("won't know where you left off reading,...")? If yes, maybe it would be worth it to include it in its documentation.

FrostyX commented 1 year ago

Simply hide the markers by adjusting their appearance.

This is easy to do:

(set-face-attribute
 'ement-room-read-receipt-marker nil
 :background (face-attribute 'default :background))

but when moving the cursor up and down it still goes through this invisible line which is not great.

alphapapa commented 1 year ago

If you use the appropriate command to move between next/previous message event, it should skip over those.

FrostyX commented 1 year ago

That makes sense, but I meant going lines up and down in Evil mode (using j, k).

alphapapa commented 1 year ago

@FrostyX Given the concerns I mentioned, I don't want to merge this PR. Would you mind opening an issue to discuss the issues you'd like to address? We can probably find solutions to them that will be relatively simple and not affect the internal operation of the markers and receipts.

FrostyX commented 1 year ago

Hello @alphapapa, sorry for my inactivity.

I created #168 from this.