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

Support access to std::fmt::Formatter inside display for more flexibility #13

Open
colin-kiegel opened this issue Dec 27, 2015 · 4 comments

Comments

@colin-kiegel
Copy link
Contributor

It would be nice to implement display via direct access to the std::fmt::Formatter - as another option. This would allow additional logic inside the display logic without temporary buffers.

Use case: Complex enum-variant with Option<...> fields.

At first I was thinking about closures, but this would require to capture all fields explicitly - not so nice. Instead I suggest we provide it like display(f, me) -> { /* ... */ }.

Suggested Grammar (example)

display(me, f) -> {
    try!(write!(f, "{} Expected token {:?} but found {:?} at {:?}.",
        me.description(), expected, found.token(), found.position()));

    // additional logic

    Ok(())
}

Current Workaround - needs redundant allocation for temporary buffer

display(me) -> ("{buf}", buf={
    use std::fmt::Write;
    let mut f = String::new()
    write!(f, "{} Expected token {:?} but found {:?} at {:?}.",
        me.description(), expected, found.token(), found.position()).unwrap();

    // additional logic

    f
}

EDIT:

  • changed argument order in display.
@tailhook
Copy link
Owner

Well, at least display() should have the arguments in the right order (same as in trait).

But your examples do not demonstrate real purpose of the thing. There should be no additional logic in the display handler, except displaying the data.

Also I'm not inclined to add every feature that may ever be possible to do here. We need only about 80%, and I think we have already covered about 99% :)

May be we should have a way to opt-out the Display implementation here, so you can implement the trait yourself?

@colin-kiegel
Copy link
Contributor Author

You are right about the argument order.

The purpose in this case would be to write some additional data only in some cases, like

if let Some(x) = reason {
    try!(write!(f, " {}", x));
}

At least that was my use case - I agree, it is not super important.

@jethrogb
Copy link

jethrogb commented Oct 26, 2017

Also want this in two different cases:

  1. for formatting ManyErrors(errors: Vec<String>) (e.g. newline-separated) without additional allocation.

  2. for using temporary variables, I don't think I can write this without a Box or without declaring a binding before the write! invocation:

My(e: MyError) {
	display("{}", match e {
		&MyError::DontDisplay{..} => &e.description() as &fmt::Display,
		_ => e as _,
	})
}

@tailhook
Copy link
Owner

tailhook commented Nov 2, 2017

@jethrogb
Well, for the first one I would like to have a crate that does Join(errors, ", ") (i.e. a wrapper type that implements Display). It has lot's of use cases and is quite common I think (is there such crate?).

For the second one, yes, I think it might be helpful. Would you like to make a PR?

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

3 participants