Tweak bril-rs library

[nwtnni]

Hello,

I'm hoping to work through the self-guided course over the next few weeks, and found some minor things in bril-rs that I would write differently. I can revert any of the changes I've made here as well!

Thanks,
Newton

[Pat-Lafon]

Hey, while we are fixing things, can I hijack this to also get rid of the double and in the docs rust.md line 20? (My bad)

Its personal preference but I would disagree with 466490b. One of the listed reasons for #[serde(rename)] is specifically to handle the case of reserved Rust keywords so that you don't need to use raw identifiers. I'm amused to find a serde issue with this exact example. Clippy unfortunately doesn't have a preference on this but at least four people on r/rust might agree with me.

We should probably check with @sampsyo that there are no plans to add a String extension/String literal to Bril. A Literal::String(String) would not be copy-able so bril-rs would end up with a rather unnecessary breaking change if it reverts that trait.

Everything else looks like a clear improvement.

[sampsyo]

Yo, @Pat-Lafon—feel free to open a mini-PR just for that. 😃 No shame in small PRs!

[sampsyo]

Hi, @nwtnni—great to hear from you!! I hope you're well.

I'll defer to @Pat-Lafon, who wrote this stuff, about the whole #[serde(rename)] thing. I think adding Copy to the data structures should be fine—if we ever wanted to add strings or other "large" constant data, we'd probably want to figure out some other way of storing it.

[nwtnni]

Thanks for the feedback and for creating the crate, @Pat-Lafon! And hello @sampsyo! Thanks for making the course so accessible :) I was reminded when I saw a couple links to it on reddit haha.

Its personal preference but I would disagree with 466490b. One of the listed reasons for #[serde(rename)] is specifically to handle the case of reserved Rust keywords so that you don't need to use raw identifiers.

Sure, I'll go ahead and revert that commit.

We should probably check with @sampsyo that there are no plans to add a String extension/String literal to Bril. A Literal::String(String) would not be copy-able so bril-rs would end up with a rather unnecessary breaking change if it reverts that trait.

Hmm, thanks for raising that point. I'm actually also leaning toward not deriving Copy to be conservative, since it's just a minor ergonomics thing.

[sampsyo]

LGTM. Happy to merge further PRs from either of y'all, of course.