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 shorthand formatting for snafu(display) #310

Merged
merged 3 commits into from
Sep 29, 2021
Merged

Add shorthand formatting for snafu(display) #310

merged 3 commits into from
Sep 29, 2021

Conversation

shepmaster
Copy link
Owner

Closes #309

@netlify
Copy link

netlify bot commented Sep 27, 2021

✔️ Deploy Preview for shepmaster-snafu ready!

🔨 Explore the source changes: 36fccba

🔍 Inspect the deploy log: https://app.netlify.com/sites/shepmaster-snafu/deploys/6152603244e4b40008ed02c1

😎 Browse the preview: https://deploy-preview-310--shepmaster-snafu.netlify.app


#[snafu(display("Person {name} with ID {id} denied", id = 99))]
RedefinedNamedArguments { id: i32, name: &'static str },
}
Copy link
Collaborator

@Enet4 Enet4 Sep 27, 2021

Choose a reason for hiding this comment

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

Just wondering, how hard would it be to support shorthand formatting inside documentation comments?

Suggested change
}
/// Person {name} with ID {id} denied
ArgumentsInComments { id: i32, name: &'static str },
}

Copy link
Owner Author

@shepmaster shepmaster Sep 27, 2021

Choose a reason for hiding this comment

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

how hard would it be

I don't think it's a one-line change, but it shouldn't be too hard; something like moving the parsing around and reusing the same format string inspection code to get the shorthand names.

More importantly, it seems... suspicious... that you'd want things like {user} in your user-facing documentation. Why do you want to show that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

More importantly, it seems... suspicious... that you'd want things like {user} in your user-facing documentation. Why do you want to show that?

Well, for one, these fields are generally as visible as the enum variants themselves, so it does not seem far out to make the message template an integral part of the documentation itself. Even without this feature, it would not feel out of place to document a variant like this:

    /// The person called `name` with the ID `id` was denied
    ArgumentsInComments { id: i32, name: &'static str },

Another concern is that if someone was already using doc comments, adding or replacing variants with parameters which should be presented to the user still requires writing #[snafu(display(...))], thus creating a mixture of doc comments and attributes in an error type (example).

Copy link
Owner Author

Choose a reason for hiding this comment

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

Updated -- thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wonderful, looks great! Thank you!

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.

Support shorthand interpolation syntax for #[snafu(display(...))]
2 participants