Skip to content

Struct attribute handle#16

Closed
jeizsm wants to merge 5 commits intojbaublitz:masterfrom
jeizsm:struct-attr
Closed

Struct attribute handle#16
jeizsm wants to merge 5 commits intojbaublitz:masterfrom
jeizsm:struct-attr

Conversation

@jeizsm
Copy link
Contributor

@jeizsm jeizsm commented Oct 24, 2018

Handle struct attribute. So it is not needed to add attribute to every field in struct, what can be cumbersome in big structs.
And multiple refactors to remove duplication mostly

src/lib.rs Outdated
extern crate getset;

#[derive(Getters, Setters, MutGetters, Default)]
#[get = "pub"] #[set = "pub"] #[get_mut = "pub"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you instead add another example that explicitly describes how the struct-level option interacts with field level options. I think this can highlight the feature and caveats better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think compile_fail example perfect for that

assert_eq!(*foo.private(), 2);
}
```
```compile_fail
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs some related documentation or something. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added documentation about struct level atrributes here

Copy link
Collaborator

@Hoverbear Hoverbear Nov 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally, it seems to make sense that this could work without erroring. If the field attributes take precedence can we implement them and not the struct level value, instead of erroring? It seems like a more flexible API.

If not, I think the compile_fail could be reduced to this:

    #[derive(Getters, Default)]
    #[get = "pub"]
    pub struct Foo {
        #[get] // Invalid!
        private: i32,
        public: i32,
    }

Then maybe we can have a full, valid example of the struct level attributes and field level working?

Sorry to be such a stickler about this. 🤣

Copy link
Contributor Author

@jeizsm jeizsm Nov 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's compile_fail test not because we cannot mix struct and field level attributes. It's fine.
But generated getter is private, and we try to call it and this is compile time error.
I clarify this is with comment

Sorry to be such a stickler about this

It's fine

@Hoverbear
Copy link
Collaborator

Hi @jeizsm !

Thanks so much for doing some cleaning! :)

I think this is a reasonable feature request and am in favor of merging it.

I'd ask you to improve the documentation around the feature this adds, and allow the get(vis = "pub") feature to happen in a different PR.

@Boscop, PTAL

@Hoverbear
Copy link
Collaborator

Closing in favor of #24. :)

@Hoverbear Hoverbear closed this Mar 7, 2019
@Hoverbear
Copy link
Collaborator

This feature is now merged, thanks for the contribution!

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 this pull request may close these issues.

2 participants