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

List of potential derives #25

Open
6 of 9 tasks
cbreeden opened this issue Jun 1, 2017 · 17 comments
Open
6 of 9 tasks

List of potential derives #25

cbreeden opened this issue Jun 1, 2017 · 17 comments

Comments

@cbreeden
Copy link

cbreeden commented Jun 1, 2017

I really like this crate, and would love to see one crate that did what you would expect for std traits. So I was thinking about compiling a list of other potential derives:

Conversions:

  • AsMut
  • AsRef

Borrowing:

  • Borrow
  • BorrowMut
  • ToOwned

Ops:

  • Index, IndexMut
  • Deref, DerefMut

Formatting:

  • Display
  • Binary, Octal, LowerHex, UpperHex, LowerExp, UpperExp, Pointer (by generalizing Display implementation)

Any thought about these? I wouldn't mind hashing out some of the details and even contributing.

@cbreeden
Copy link
Author

cbreeden commented Jun 1, 2017

It's unclear to me how one would implement the ones I crossed off. One option would be to just allow one to pass in the type parameter via a string like: #[Derive(Index)] #[Index("usize")] assuming the inner type satisfies Index<usize>. But this seems very unhygienic and might be worth waiting for later.

@JelteF
Copy link
Owner

JelteF commented Jun 1, 2017

Hi Christopher, thanks for the suggestions. I don't think Display should ever output debug output. That division is there for a reason. Furthermore, I don't think there's a general enough solution for displaying a struct with multiple fields. Structs with a single field would make sense though. The same I think is true for all of the other traits you mention (except maybe deref).

So deriving these for newtypes should be quite easy. Even the ones you crossed off should be possible, because you can use generics there (see the Mul like traits for an example). As a next step it would be cool to be able to mark one of the fields of a multi field struct with #[Display] to indicate that it should forward display to that field.

I don't really have time to implement these features in the near future though. If you would like to try it's fine though and I'll try to free up some time to merge a PR.

@JelteF
Copy link
Owner

JelteF commented Mar 16, 2018

Hey @cbreeden, I changed your comment at the top a bit for an easy checklist. I hope you're fine with that. Display is now implemented on master for newtypes btw.

@Boscop
Copy link

Boscop commented Mar 26, 2018

Please add Deref/DerefMut like https://crates.io/crates/shrinkwraprs

@JelteF
Copy link
Owner

JelteF commented Mar 26, 2018

I have a deref branch at the moment (not merged because I haven't made the docs yet), but it is quite different from shrinkwraprs. It will just forward the operations to the member. I like the shrinkwraprs idea, but I don't think it makes sense to have this behaviour as the default. You can still use shrinkwraprs combined with derive_more if you want its behaviour for Deref/DerefMut.

@Boscop
Copy link

Boscop commented Mar 26, 2018

I haven't used shrinkwraprs before, how is its Deref impl different to your approach?

@JelteF
Copy link
Owner

JelteF commented Mar 28, 2018

I've just released v0.10.0 with Deref and DerefMut support. Example usage and generated code can be found here:
https://jeltef.github.io/derive_more/derive_more/deref.html
https://jeltef.github.io/derive_more/derive_more/deref_mut.html

I think those links should show the difference in usage with this: https://crates.io/crates/shrinkwraprs

@Boscop
Copy link

Boscop commented Mar 29, 2018

Does it only work for members that also impl Deref?

@agausmann
Copy link

Any further discussion needed before AsRef and AsMut are implemented? Newtypes are pretty straightforward, but if we want to do it for other structs, it's not as easy and would probably require parameterization of the desired fields - otherwise it won't know how to handle multiple fields of the same type. I wouldn't know how to do that, but I may be able to contribute the newtype definition.

@reynoldsbd
Copy link
Contributor

@agausmann for structs with multiple fields, I think it would make sense for each field to "opt in" to the AsRef derivation via an attribute. Something like this:

#[derive(AsRef)]
struct Foo {

    #[as_ref]
    bar: Bar,
    // Generates impl AsRef<Bar> for Foo { ... }

    #[as_ref]
    baz: Baz,
    // Generates impl AsRef<Baz> for Foo { ... }
}

I don't think it's possible to implement AsRef multiple times for different fields of the same type. For instance, trying to derive AsRef as follows should result in an error:

#[derive(AsRef)]
struct Foo {

    #[as_ref]
    bar1: Bar,

    // Error! Can't implement AsRef<Bar> multiple times
    #[as_ref]
    bar2: Bar,
}

@JelteF would you be willing to accept a PR that adds AsRef and AsMut support as described above?

@JelteF
Copy link
Owner

JelteF commented Sep 11, 2019 via email

@agausmann
Copy link

I'd avoid using multiple top-level attributes. It would be better to use a single attribute name like #[derive_more(as_ref, as_mut)], similar to the pattern that Serde uses.

@JelteF
Copy link
Owner

JelteF commented Sep 11, 2019

@agausmann on one hand that makes sense, but we already do that for the display style attributes. So I think it's okay to have them as top level ones to stay consistent. Furthermore I'm not sure if you can provide arguments to non top level atributes, which would probably be nice given this issue: #60 There you could use #[from(ignore)]

@tyranron
Copy link
Collaborator

@JelteF is #[derive(Error)] would be desirable? I've tried several err derive crates out there and they provide custom display capabilities for ergonomics, which, however, do not mix with our #[derive(Display)] well.

What I propose is to add simple #[derive(Error)] to derive_more which will derive only std::error::Error implementation. Without any implicit From and Display implementations. This will play very well with existing #[derive(Display)] and #[derive(From)] macros.

If it's acceptable, I'll roll-out implementation in a few working days.

@JelteF
Copy link
Owner

JelteF commented Oct 11, 2019 via email

@JelteF JelteF mentioned this issue Oct 12, 2019
@wycats
Copy link

wycats commented Sep 29, 2020

I ran into a situation where I'd love to be able to use the formatting system in #[derive(Display)] with Debug (for situations where not all of the fields are Debug). Does this make sense?

@JelteF
Copy link
Owner

JelteF commented Sep 29, 2020

You should be able to use #[derive(DebugCustom)] for that.

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

No branches or pull requests

7 participants