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

relay: Add "sync * hotlist" option #1724

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

nilscc
Copy link

@nilscc nilscc commented Dec 1, 2021

Hi,

this PR adds an option for a relay client to sync on the "hotlist_changed" hook signal.

Please review and let me know if anything should be changed.

TODO: Update documentation.

@nilscc nilscc changed the title New feature for relay: Add "sync * hotlist" option relay: Add "sync * hotlist" option Dec 1, 2021
@flashcode flashcode self-assigned this Dec 1, 2021
@flashcode flashcode added the feature New feature request label Dec 1, 2021
Copy link
Member

@flashcode flashcode left a comment

Choose a reason for hiding this comment

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

Apart from the small point noted in the comment, seems OK for me!
I'll update the docs in a separate commit.

src/plugins/relay/weechat/relay-weechat-protocol.c Outdated Show resolved Hide resolved
@nilscc nilscc requested a review from flashcode December 1, 2021 18:15
@codecov-commenter
Copy link

Codecov Report

Merging #1724 (36123b3) into master (d447755) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1724      +/-   ##
==========================================
- Coverage   37.26%   37.25%   -0.02%     
==========================================
  Files         209      209              
  Lines       84606    84633      +27     
==========================================
  Hits        31527    31527              
- Misses      53079    53106      +27     
Impacted Files Coverage Δ
src/plugins/relay/weechat/relay-weechat-protocol.c 0.00% <0.00%> (ø)
src/plugins/relay/weechat/relay-weechat.c 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d447755...36123b3. Read the comment docs.

@nilscc nilscc marked this pull request as draft December 1, 2021 22:31
@nilscc
Copy link
Author

nilscc commented Dec 1, 2021

PR converted to draft as I'm observing weechat crashes with these changes. I'm investigating.

@nilscc nilscc marked this pull request as ready for review December 7, 2021 20:03
@2xsaiko
Copy link

2xsaiko commented Nov 11, 2022

Would this allow for sending the hotlist state from the relay to the clients, so kind of the reverse of what is already there with clients sending their read status?

@nilscc
Copy link
Author

nilscc commented Feb 24, 2023

@2xsaiko yeah, sure. it saves you from syncing on all buffers while still giving you notice of highlights etc.

@nilscc
Copy link
Author

nilscc commented Nov 29, 2023

@flashcode What would be missing to get this change merged?

@flashcode
Copy link
Member

We have to check if the add of line hdata could be problematic for existing clients. It should not, but it depends how the clients implement the protocol, it can break.

Once we validated the major relay clients are OK with this change, this can be merged (I'll add appropriate documentation as well).

Did you test some relay clients with these changes?

@flashcode flashcode added the waiting info Waiting for info from author of issue label Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request waiting info Waiting for info from author of issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants