search

rust-cli / human-panic

Panic messages for humans.
https://docs.rs/human-panic
Apache License 2.0
1.66k stars 65 forks source link

Tracking Issue: v2.0.0 #46

Open spacekookie opened 6 years ago

spacekookie commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

Yay a :bike: :house: !

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

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 :slightly_smiling_face: My 2-cents.

yoshuawuyts commented 6 years ago

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

spacekookie commented 6 years ago

:tada:

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 commented 5 years ago

I just branched off fed6fda1a4827c00e9c43e580f2ea20ed9ee9b0b 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 commented 5 years ago

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