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

MUC MAM #1862

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

MarcoPolo-PasTonMolo
Copy link
Collaborator

@MarcoPolo-PasTonMolo MarcoPolo-PasTonMolo commented Jul 1, 2023

MUC MAM

See #660

How to test the functionality

  • step 1: Have mam enabled
  • step 2: Join random muc scroll up
  • step 3: rejoining muc should fetch messages sent since last online with profanity

I ran valgrind when using my new feature

no


TODO

  • Look up older history
  • Look up history when creating muc window
  • Look up newer history
  • Fetch mam on incoming message (does a new muc even get created on incoming message ?)
  • Fetch mam on reconnects
  • Scroll history even without mam
  • Why loading message sometimes lingers? (when at the end of muc mam maybe?)
  • Why is a chatwin opened for each member of muc?
  • Last messages appear doubled (maybe this only happens for chatwins?)
  • wrong timestamp for messages through mam on new join of muc
  • mentions/triggers show printed date not original date
  • respect mam setting
  • get all rsm results even from pagination
  • Autocompletion has to be correct
  • how to handle read indicator with mam
  • valgrind
  • make loading mam for muc faster if possible
  • Don't request history from muc

TODO split this commit later and see if all its changes are needed

See profanity-im#660
@MarcoPolo-PasTonMolo MarcoPolo-PasTonMolo changed the title Implement basic mam for mucs MUC MAM Jul 1, 2023
@jubalh jubalh added this to the next milestone Jul 2, 2023
jubalh added a commit that referenced this pull request Jul 3, 2023
Started in #1306
for 0.9.0 by me.
We mentioned that it was experimental.

Later heavily improved in
#1724
for upcoming version by MarcoPolo-PasTonMolo.

MUC MAM is in development at:
#1862

Tracking issue: #660
Show correct timestamp for mentions and triggers (it used to show
current timestamp instead of the message's one), and flip commands when
displaying history.

See profanity-im#660
@jubalh
Copy link
Member

jubalh commented Jul 18, 2023

Fetch mam on incoming message (does a new muc even get created on incoming message ?)

I'm not sure I understand correctly. But: I think MAM only needs to be fetched for MUCs that are currently joined? Windows are only open for currently joined MUCs.

@MarcoPolo-PasTonMolo
Copy link
Collaborator Author

I'm not sure I understand correctly. But: I think MAM only needs to be fetched for MUCs that are currently joined? Windows are only open for currently joined MUCs.

I remember that chatwins are created/opened when the other party sends a message. I was thinking that maybe that was also happening for MUCs and so I would have to handle MAM in the case that the user is not the one that opens a mucwin by /joining it

@ike08
Copy link
Contributor

ike08 commented Oct 17, 2023

@MarcoPolo-PasTonMolo, do you want help with this?

This commit should not go into production, its all testing, might or
might not work. Mapped pagedown to cause a disconnect so I could easily
test MAM on reconnect.
@MarcoPolo-PasTonMolo
Copy link
Collaborator Author

@MarcoPolo-PasTonMolo, do you want help with this?

Yes please. I don't have time to work on this. My code can be pretty garbage at times especially in iq.c so if you have any questions ask me. I think I would respond quicker in xmpp so Ill tag you there.

If I remember correctly MAM for MUCs is pretty much done. I had started implementing MAM for when a user gets reconnected but hit a few road blocks like how do I prevent new messages, that arrive while fetching mam after a reconnect, from getting jumbled up with the old ones from MAM. Or how do I load the actual MAM messages while also giving some feedback to the user so he doesn't think that the ui is stuck (like use pagination and fetch 10 messages at a time etc)

I mapped pagedown to trigger a disconnect so that testing is easier with a /reconnect interval of 5 seconds.

Here's what I had written down as TODO

== Reconnect mam ==

  • Fetch more messages if not all fetched
  • Make it work
  • refactor code to be cleaner and more understandable

= MUC MAM =

  • Look up older history
  • Look up history when creating muc window
  • Look up newer history
  • lost connection while loading messages
  • Fetch mam on reconnects
  • Why loading message sometimes lingers and doesn't disappear? (when at the end of muc mam maybe?)
  • how to handle read indicator with mam
  • Last messages appear doubled (maybe this only happens for chatwins?) when i get a new message from the contact while window is not open
  • Autocompletion has to be correct
  • make loading faster (or somehow show progress)
  • Fetch mam on incoming message (does a new muc even get created on incoming message ?)
  • Scroll history even without mam
  • Why is a chatwin opened for each member of muc?
  • wrong timestamp for messages through mam on new join of muc
  • mentions/triggers show printed date not original date
  • respect mam setting
    • not showing history when mam is off,
    • and after first join and fetch of messages
  • get all rsm results even from pagination
  • valgrind
  • see if all added code is neccessary
  • split first commit
  • does it load multiple (does pagination work)? or did we distroy it with return 0 in rsm handler?
  • no nick in front of messages in muc mam
  • auto_chars

@jubalh
Copy link
Member

jubalh commented Oct 19, 2023

Thanks for your work and the summary @MarcoPolo-PasTonMolo !!

@jubalh
Copy link
Member

jubalh commented Jun 2, 2024

@ike08 @MarcoPolo-PasTonMolo slowly I have more free time to work on profanity again :) I was curious whether the same is true for one of you guys and whether you are still interested in implementing this feature. Cheers!

@MarcoPolo-PasTonMolo
Copy link
Collaborator Author

Not much time sorry

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

Successfully merging this pull request may close these issues.

None yet

3 participants