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

Tracking Issue: v2.0.0 #46

Open
4 of 7 tasks
spacekookie opened this issue Sep 26, 2018 · 8 comments
Open
4 of 7 tasks

Tracking Issue: v2.0.0 #46

spacekookie opened this issue Sep 26, 2018 · 8 comments
Labels
enhancement Improve the expected
Milestone

Comments

@spacekookie
Copy link
Collaborator

spacekookie commented Sep 26, 2018

This is a work-in-progress tracking issue for a 2.0 release of human-panic, set in motion because of #45

New Features

@yoshuawuyts
Copy link
Collaborator

Something that might be useful to bikeshed for a 2.0 release is renaming setup_panic!(). It's seemed a bit odd to me given we don't quite setup a panic, but we setup a panic "handler" -- which seems quite different.

I'm thinking something like catch_panic!() or catch_unwind!(). The latter is in line with the standard library's naming scheme, which might make it easier to remember also.

@mathstuf
Copy link
Contributor

mathstuf commented Oct 3, 2018

How about install_human_panic!()?

The catch_ macros seem like they should "wrap" around some block to catch a panic within them. human-panic installs a global panic handler instead.

@yoshuawuyts
Copy link
Collaborator

@mathstuf with the 2018 module system macros should become a bit more easily traceable to their source. E.g.

import macro to file

use human_panic::install_human_panic;

fn main () {
  install_human_panic!();
}

use macro from import

use human_panic;

fn main () {
  human_panic::install_human_panic!();
}

It feels like in both cases the word human_panic is repeated a lot. I feel like having something without that name might make it a bit clearer.


The catch_ macros seem like they should "wrap" around some block to catch a panic within them. human-panic installs a global panic handler instead.

My personal preference would be to adopt human_panic::catch_unwind!();, which is similar to the stdlib's panic::catch_unwind!() method:

stdlib

fn main () {
  panic::catch_unwind(|| println!("hello!"));
}

human_panic

fn main () {
  human_panic::catch_unwind!();
}

There's probably some compat issues to be figured out, but I'm thinking the general idea should hold.

@spacekookie
Copy link
Collaborator Author

Yay a 🚲 🏠 !

Renaming the setup macro is definitely a good idea, although I'm not sure I'm happy with any of the suggestions so far 😉

Repeating the term human_panic over and over is really not helpful whereas human_panic::catch_unwind!() looks too much like there is immediate action in the macro (or it's expecting to wrap some code – the way that human-panic v0.1 worked).

Instead what about something like this.

fn main() {
  human_panic::panic_handler!();

And it's still clear enough when the macro is directly in scope

use human_panic;
fn main() {
  panic_handler!()
}

It signals that some panic_handler action is performed, but I think is also clear enough that no immediate action is required. Plus it doesn't repeat human_panic twice 🙂 My 2-cents.

@yoshuawuyts
Copy link
Collaborator

@spacekookie yeah that seems reasonable! -- and yeah I guess we do have (our first?) bike shed on our hands 🎉 wooh; that's not a bad problem to have!

@spacekookie
Copy link
Collaborator Author

🎉

So #48 is now on master and if you look at the PR #50 we might want to release this only under a 2.0.0 because it can have severe impact on external systems. So that might be another good reason to make the next version a major bump.

@spacekookie spacekookie added this to the 2.0.0 milestone Oct 5, 2018
@spacekookie spacekookie added the enhancement Improve the expected label Oct 5, 2018
@spacekookie spacekookie pinned this issue Jun 14, 2019
@spacekookie spacekookie changed the title 2.0 tracking issue Tracking Issue: v2.0.0 Jun 14, 2019
@spacekookie
Copy link
Collaborator Author

I just branched off fed6fda into it's own branch: 1.0.0-maintenance which is where all bug fixes for the 1.0.0 compatible versions can be accumulated.

This means that master is now breaking-changes land for v2.0.0!

@MichaelMcDonnell
Copy link

Should custom error messages (issue #54) be part of v2.0.0? It is the biggest thing holding me back from using human-panic.

fnichol added a commit to fnichol/iocage-provision that referenced this issue Oct 10, 2019
This change reverts the codebase to use the vanilla upstream version of
`human-panic` so this tool can be published on Crates.io in the
meantime. Unfortuntely, this adds multiple crates which are **not** used
and are purely extra.

When some of the upstream PRs and issues are resolved, merged, and
released, then this commit may become revertable.

References: rust-cli/human-panic#46

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>
fnichol added a commit to fnichol/iocage-provision that referenced this issue Oct 10, 2019
This change reverts the codebase to use the vanilla upstream version of
`human-panic` so this tool can be published on Crates.io in the
meantime. Unfortuntely, this adds multiple crates which are **not** used
and are purely extra.

When some of the upstream PRs and issues are resolved, merged, and
released, then this commit may become revertable.

References: rust-cli/human-panic#46

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve the expected
Projects
None yet
Development

No branches or pull requests

4 participants