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

please add getters to all settable things #619

Open
pm100 opened this issue Nov 8, 2023 · 5 comments
Open

please add getters to all settable things #619

pm100 opened this issue Nov 8, 2023 · 5 comments
Labels
Status: Abandoned Incomplete work that can be picked up if needed Type: Enhancement New feature or request

Comments

@pm100
Copy link
Contributor

pm100 commented Nov 8, 2023

Problem

My immediate example is that I need to look inside a Block being passed to me (I am a library that a tui app uses). I need to read the border_style and title. But I think this is a general point, especially for a library that adds tui features (tui_textarea in my case)

It would be nice if rust had c# style properties, but it does not, so this is a lot of sloggy typing

Solution

Add get_xxx fn to every struct that has an xxx fn that sets the xxx property.

Alternatives

@pm100 pm100 added the Type: Enhancement New feature or request label Nov 8, 2023
@joshka
Copy link
Member

joshka commented Nov 8, 2023

This has come up before (#453 and #78 are worth a quick read). I think this use case makes a much better argument for why we need to add getters than for unit testing. I'm not sure whether to consolidate this into the discussion on #78 / #453 or whether to treat it as a new thing do you think your request is enough alike #78 to be useful there - if so, let's copy it over and close this?

There aren't C# style getters in rust, mainly because idiomatically rust types that need them tend to just make the fields public - which might be the right option here (particularly if we mark the types non-exhaustive). The alternatives are crates that macro up getters like https://crates.io/crates/derive-getters or https://crates.io/crates/getset

@pm100
Copy link
Contributor Author

pm100 commented Nov 8, 2023

#78 is much larger in scope so, no, please don't merge this. This one is fundamentally simple

Would you accept a PR using derive-getters?

(Currently disappearing down a open source rabbit hole, originally working on gitui, it needs textarea, turns out that needs tweaks, turns out those fixes need tui changes..just posted issue to windows terminal needed by crossterm.. happy days)

@joshka
Copy link
Member

joshka commented Nov 8, 2023

(Currently disappearing down a open source rabbit hole, originally working on gitui, it needs textarea, turns out that needs tweaks, turns out those fixes need tui changes..just posted issue to windows terminal needed by crossterm.. happy days)

Yak shaving is a funny business.

Would you accept a PR using derive-getters?

I think this would be good to see.

Because the builder methods currently use the field name (e.g. Block::title()) you'll need to add an attribute on every field to rename the getter. I suspect getset will be easier. E.g:

#[derive(Debug, Default, Clone, Eq, PartialEq, Hash, Getters)]
#[getset(get = "pub with_prefix")]

Make sure to pay some attention to how the API docs render. I suspect the field level comments will not be good enough for the getters in some cases as we've often commented just the builder methods. Perhaps it's sufficient to just link to the setters from each field?

@pm100
Copy link
Contributor Author

pm100 commented Nov 8, 2023

Yup I saw that about names. But I like getters simplicity, you can just ask to generate all getters with a derive at the struct level. So now my next rabbit hole to get derive-getters to accept a prefix attribute at the struct level

I just found out that it's not on GitHub so now I have to learn a different repo / issue / pr system. Sigh

@pm100
Copy link
Contributor Author

pm100 commented Nov 8, 2023

well i see that getset has the same feature, so thats good.

FYI. The ultimate reason I am doing this is to get rid of 'tick infection'. Because Block is in fact Block<>, anything that embeds it has to also be <> and that ripples up the entire stack of containment. This is annoying when trying to do a drop in replacement into a tui app that is tick free. Block has it because it doesnt want to own its title strings . Being able to read a tui block into my own type allows me to embed my own type instead of the supplied Block argument , and then reconsitute it later once needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Abandoned Incomplete work that can be picked up if needed Type: Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants