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

Trait implementations for serialization #19

Closed
samtay opened this issue Jun 10, 2020 · 5 comments · Fixed by #53
Closed

Trait implementations for serialization #19

samtay opened this issue Jun 10, 2020 · 5 comments · Fixed by #53

Comments

@samtay
Copy link

samtay commented Jun 10, 2020

I'd like to have my MadSkin come from a user-supplied theme, like colors.yml. I don't mind just making a newtype around MadSkin to do so, but I noticed the code in broot that handles parsing some user-friendlier keywords, etc., and just wanted to check if you plan on moving that back out to minimad or termimad.

I also thought, to minimize effort, perhaps you'd be interested in just adding a package flag serialization which if set, would add serde dependency and provide serialize/deserialize traits. (To avoid users having to deal with orphan problems when they want to have theme configuration.)

Just opening this issue for your awareness; if you don't feel that serialization should be a part of the library, feel free to close it! Thanks.

@Canop
Copy link
Owner

Canop commented Jun 10, 2020

This is a good question.

While I considered having a library or feature for the (de)serialization of a whole madskin, which is a quite complex problem in reality as you typically want to deal with several of them and you also want to deal with your hadcoded ones (hence the StyleMap! macro in broot), I didn't think about making it easier to deal with your own madskin-like types by first offering (de)serialization at the compund_style level.

It should be noted that termimad's CompoundStyle is nothing more than a Crossterm ContentStyle. Rather than putting this (de)serialization utility in termimad, it could be offered in an independant library which would be available to crossterm users without forcing them to use termimad. I don't know how many users would be interested but the initial effort seems quite low (pinging Timon).

@Canop
Copy link
Owner

Canop commented Jun 11, 2020

Reading this again, I think I should just add the "serde" feature and derive (de)serialize when the feature is active.
Not so easy, crossterm's StyledContent<char> doesn't derive Serialize and Deserialize so there's more work to do.

@sevenuz
Copy link

sevenuz commented Oct 20, 2023

Hallo, I am also interested in a serialized form of the skin. Why is it possible to deserialize it, as it is done in your example skin-file, but not possible to derive serialize?
Sorry I am quite new to Rust and not sure, if this question makes sense :D

@Canop
Copy link
Owner

Canop commented Oct 20, 2023

@sevenuz Yes, your question makes sense.

On the surface, the reason you can't just derive Serialize is that you need the inner properties to implement Serialize too.

But it doesn't mean I can't implement Serialize, it's just that I didn't initially see the need and then I forgot this issue. I'm putting it in my TODO list now.

@sevenuz
Copy link

sevenuz commented Oct 21, 2023

Okey, that sounds promising. As a workaround I will use the a default skin to serialize, then the user can edit manually and I can deserialize as it is done in your example, so this has no priority :D

@Canop Canop mentioned this issue Nov 2, 2023
@Canop Canop closed this as completed in #53 Nov 3, 2023
Canop added a commit that referenced this issue Nov 3, 2023
There was already a `serde::Deserialize` implementation for `MadSkin`.

There's now a `serde::Serialize` implementation.

Fix #19
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 a pull request may close this issue.

3 participants