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

Add get_option attribute #82

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

hochej
Copy link

@hochej hochej commented Feb 2, 2022

Implements #78

Hi @Hoverbear, first of all thanks for your work!
I stumbled across the same issue as mentioned in #78 and tried to implement the suggested solution.
This PR adds a new attribute get_option to getset, so that the getter function returns Option<&T> instead of &Option<T>.

Note the following syntaxes are supported (and tested) for Options:

  • Option
  • std::option::Option
  • ::std::option::Option
  • core::option::Option
  • ::core::option::Option

However own type aliases like type Bar<T> = Option<T> are not supported.

The following code:

use getset::{OptionGetters};
#[derive(OptionGetters)]
#[getset(get_option)]
pub struct Foo {
    field: Option<usize>,
}

derives

impl Foo {
    #[inline(always)]
    fn field(&self) -> Option<&usize> {
        self.field.as_ref()
    }
}

Unfortunately, I have little experience with procedural macros so far and hope that I have implemented the suggestion in an acceptable way.

Getters that return `&Option<T>` instead of `Option<&T>` can lead to boilerplate code, as they require the user to pollute code with `.as_ref()`.
Previously, even in cases where a field did not have a get_option
attribute, an attempt was made to determine the inner type of the
option. This has erroneously led to errors.
The change now ensures that the implement function is returnes
beforehand.
@hochej
Copy link
Author

hochej commented Feb 2, 2022

I noticed that the same behavior, as described in #74, occurs in this case.
This means it is not possible to use field level attributes to override the global attribute for get, get_copy or get_option.

@Hoverbear
Copy link
Collaborator

Yeah, I'm kind of convinced global options was a mistake. :(

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