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

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

Open
CobaltCause opened this issue Nov 30, 2020 · 12 comments
Open

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

CobaltCause opened this issue Nov 30, 2020 · 12 comments

Comments

@CobaltCause
Copy link

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.

@poljar
Copy link
Owner

poljar commented Nov 30, 2020

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.

@CobaltCause
Copy link
Author

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, rust-lang/rust#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.

@poljar
Copy link
Owner

poljar commented Dec 1, 2020

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.

@CobaltCause
Copy link
Author

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

@poljar
Copy link
Owner

poljar commented Dec 2, 2020

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

@CobaltCause
Copy link
Author

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.)

@poljar
Copy link
Owner

poljar commented Dec 2, 2020

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.

@CobaltCause
Copy link
Author

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?

@poljar
Copy link
Owner

poljar commented Dec 2, 2020

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.

@CobaltCause
Copy link
Author

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

@Noskcaj19
Copy link

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.

@poljar
Copy link
Owner

poljar commented Dec 2, 2020

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.

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

No branches or pull requests

3 participants