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

Update rust-weechat & matrix-sdk #86

Merged
merged 11 commits into from
Oct 14, 2024
Merged

Update rust-weechat & matrix-sdk #86

merged 11 commits into from
Oct 14, 2024

Conversation

panekj
Copy link
Contributor

@panekj panekj commented Sep 2, 2024

@panekj
Copy link
Contributor Author

panekj commented Sep 2, 2024

image

@panekj

This comment was marked as resolved.

@panekj

This comment was marked as resolved.

@panekj

This comment was marked as resolved.

@poljar
Copy link
Owner

poljar commented Sep 5, 2024

Not a real review for now, just wanted to thank you for doing this.

@panekj
Copy link
Contributor Author

panekj commented Sep 5, 2024

No problem, I created both PRs as drafts for now since I'm combing through the codebase trying to map how it works (looks clean and simple to me :) but also because I'm trying to fix the usage first; and once I'm sure that it's usable for others, I'll clean it up for proper review (I don't mind nitpicks and suggestions though).

I'm trying to use matrix solely via weechat, so far the only offenders have been BIG rooms (like #matrix:matrix.org which has 62K users; thus the d86b41d (#86) commit)

@poljar
Copy link
Owner

poljar commented Sep 5, 2024

I'm trying to use matrix solely via weechat, so far the only offenders have been BIG rooms (like #matrix:matrix.org which has 62K users; thus the d86b41d (#86) commit)

Yeah, WeeChat itself has trouble dealing with that many nicks in the nicklist, the Python version of this plugin had to introduce such a limit as well.

There are some new APIs in the Matrix Rust SDK and Matrix Spec which should help with the initial sync itself taking a lot of data, but that's a topic for the future.

@panekj

This comment was marked as outdated.

@panekj

This comment was marked as resolved.

@panekj
Copy link
Contributor Author

panekj commented Sep 13, 2024

Also since matrix.org switched to authenticated media, I'm not exactly sure how to handle the mxc URLs since putting the token in query is not advised (also super bad)

@poljar
Copy link
Owner

poljar commented Sep 14, 2024

Also since matrix.org switched to authenticated media, I'm not exactly sure how to handle the mxc URLs since putting the token in query is not advised (also super bad)

Yeah, you would probably need a small helper of some sort which knows the access token, or you could keep it in the query and the small helper puts the token into the header. Ideally we would also use the URL escape code so we hide all the unnecessary data the URL needs to contain.

@panekj panekj changed the title fix: update rust-weechat,matrix-sdk Update rust-weechat & matrix-sdk Oct 12, 2024
@panekj
Copy link
Contributor Author

panekj commented Oct 12, 2024

Need to figure what I broke or if matrix.org changed something recently because it does not load new messages, only grabbing backlog via PageUp works (and misses out on few last messages)

Fixed now

@panekj
Copy link
Contributor Author

panekj commented Oct 14, 2024

I've removed temp fixes for too many users syncing, clippy and others, making this PR mostly just a compile fix (with few replaced panics) and removing not needed dependencies.

I'll work sliding sync filters and optimisations in new PR.

In terms of duplicated dependencies:


cfg-if 0.1.10 is held back due to pipe-channel dependency
Might need to consider using bugaevc/pipe-channel#3 since the dependency looks to be not maintained anymore

cfg-if v0.1.10
└── nix v0.17.0
    └── pipe-channel v1.3.0
        └── weechat v0.4.0 (https://github.com/poljar/rust-weechat#58a836b0)
            └── weechat-matrix v0.1.0 (/var/mnt/SSD/.dev/github.com/poljar/weechat-matrix-rs)

syn 1.x.x:

  • cfg-vis should disappear with next matrix-sdk release as cfg-vis was removed from matrix-sdk
  • proc-macro-error same as above, thanks to matrix-org/matrix-rust-sdk@17370a5
  • strum_macros have to bump strum in rust-weechat
  • weechat-macro will have to look at upgrading it to syn 2
syn v1.0.99
├── cfg-vis v0.3.0 (proc-macro)
│   └── matrix-sdk v0.7.1
│       └── weechat-matrix v0.1.0 (/var/mnt/SSD/.dev/github.com/poljar/weechat-matrix-rs)
├── proc-macro-error v1.0.4
│   └── aquamarine v0.5.0 (proc-macro)
│       └── matrix-sdk v0.7.1 (*)
├── strum_macros v0.24.3 (proc-macro)
│   └── strum v0.24.1
│       ├── weechat v0.4.0 (https://github.com/poljar/rust-weechat#58a836b0)
│       │   └── weechat-matrix v0.1.0 (/var/mnt/SSD/.dev/github.com/poljar/weechat-matrix-rs)
│       └── weechat-matrix v0.1.0 (/var/mnt/SSD/.dev/github.com/poljar/weechat-matrix-rs)
└── weechat-macro v0.4.0 (proc-macro) (https://github.com/poljar/rust-weechat#58a836b0)
    └── weechat v0.4.0 (https://github.com/poljar/rust-weechat#58a836b0) (*)

bitflags 1 after updating pulldown-cmark and openssl

  • clap will have to look at if maybe clap_lex will suffice
bitflags v1.3.2
├── clap v2.34.0
│   └── weechat-matrix v0.1.0 (/var/mnt/SSD/.dev/github.com/poljar/weechat-matrix-rs)
└── nix v0.17.0
    └── pipe-channel v1.3.0
        └── weechat v0.4.0 (https://github.com/poljar/rust-weechat#58a836b0)
            └── weechat-matrix v0.1.0 (/var/mnt/SSD/.dev/github.com/poljar/weechat-matrix-rs)

@panekj panekj marked this pull request as ready for review October 14, 2024 13:56
Copy link
Owner

@poljar poljar left a comment

Choose a reason for hiding this comment

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

Ah, thanks for simplifying this, I tested this and it did work for me as well.

I think we can merge this.

@poljar poljar merged commit 8f0f981 into poljar:main Oct 14, 2024
2 checks passed
@panekj panekj deleted the fix/update branch October 14, 2024 14:03
@panekj panekj mentioned this pull request Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants