rust-weechat icon indicating copy to clipboard operation
rust-weechat copied to clipboard

relax plugin!()'s fields requirement of string literal

Open CobaltCause opened this issue 4 years ago • 12 comments

It would be nice if the name, author, and so forth could take &'static strs instead so that env!("CARGO_PKG_NAME") and friends could be used in place of duplicating that information inside the macro verbatim. I didn't look at the source code to see if this is even possible to implement so feel free to close this if it's not.

CobaltCause avatar Nov 30 '20 03:11 CobaltCause

This annoyed me too and I did take a brief look how to do this, I didn't find a clear answer so if someone figures this out before me a PR is welcome.

poljar avatar Nov 30 '20 08:11 poljar

Alright so I decided to take a shot at this and it turns out it's even more involved than I thought. I decided not to open a PR because this feature requires crates using the proc-macro to use a nightly toolchain. My code's visible here if you'd like to see my changes and that it does actually work (in particular, check out the sample example which uses the env!() macro). If you want me to open a PR despite the nightly requirement, let me know and I'll do so.

Also, https://github.com/rust-lang/rust/issues/51911 has information about the particular nightly feature being leveraged here. I tried the union trick mentioned there and ran into the "The rules on what exactly is undefined behavior aren't clear [...]" error, so either that doesn't actually work or I misunderstood how to use it.

CobaltCause avatar Dec 01 '20 05:12 CobaltCause

Feel free to open a PR so this doesn't get lost, but yeah shame that it requires nightly. Could you just add some comments explaining what's going on, I know I should have done the same when I first wrote the proc macro as well.

poljar avatar Dec 01 '20 16:12 poljar

PR is up (with a few comments added) at #29

CobaltCause avatar Dec 02 '20 01:12 CobaltCause

Thanks, we'll leave this open in the meantime. Out of interest are you building a plugin using this?

poljar avatar Dec 02 '20 16:12 poljar

Well, I was thinking about it. I've got this project that I've been working on and my current client's code is getting quite ugly, primarily due to TUI stuff, so I wanted to see how feasible it would be to rewrite it as a Weechat plugin to offload the actual UI problems to somewhere else. (I'm also active on Matrix as @charles:typ3.tech in #rust:matrix.org or DMs for synchronous discussion, if you prefer not to clutter up this issue too much.)

CobaltCause avatar Dec 02 '20 17:12 CobaltCause

It should™ be feasible though beware that Weechat doesn't support inserting new messages anywhere else besides at the bottom of a buffer, which makes stuff a bit annoying if you do need to support room history like in Matrix.

This implements a larger plugin so it might be an useful example for your project as well.

poljar avatar Dec 02 '20 17:12 poljar

doesn't support inserting new messages anywhere else besides at the bottom of a buffer

Oh, huh, that's good to know and also unfortunate. I ended up using a BTreeSet to store events in my current client so things would stay ordered/deduplicated visually when getting old events. Also thanks for the link, I'll definitely give it a look. It seems like the python version is able to load and order old events, I'm guessing this is done by clearing the buffer and rewriting everything manually?

CobaltCause avatar Dec 02 '20 18:12 CobaltCause

No, you can modify printed lines, though the highlight state can't be modified sadly. The Python version as well as the Rust version sort the messages in the buffer. Clearing and printing again is another possibility, the Weechat slack script is using that method.

I don't really know which one is faster/better.

poljar avatar Dec 02 '20 18:12 poljar

Oh, I see. Would clearing allow "modifying" the highlight state? I'd imagine in-place modification would be more performant, but really the only way to know for sure is benchmarking it somehow

CobaltCause avatar Dec 02 '20 19:12 CobaltCause

For another approach to handling message updates, I originally used an approach similar to wee-slack, however I wanted to store more advanced metadata for each message. I ended up writing a storage layer that lets me store message objects, which can then be rendered and printed. Currently it updates printed messages by clearing the buffer and reprinting everything, however it would be easy to modify the lines when possible.

Noskcaj19 avatar Dec 02 '20 19:12 Noskcaj19

Clearing would allow having the correct highlight state, yes. The patch to enable modifying the highlight state is trivial and I should try to get it merged into Weechat one of these days.

poljar avatar Dec 02 '20 19:12 poljar