search

pacak / bpaf

Command line parser with applicative interface
Apache License 2.0
340 stars 17 forks source link

Consider silent failure rather than panicking in `complete_gen.rs` #305

Closed zmitchell closed 5 months ago

zmitchell commented 11 months ago

I somehow got myself into a state where completions are panicking when I hit TAB:

thread 'main' panicked at 'Unsupported output revision 4, you need to genenerate your shell completion files for the app', /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/bpaf-0.9.5/src/complete_gen.rs:402:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

It's probably a bug on my end regarding how I got into this state, but in my opinion a silent failure is much better from a UX perspective than hitting TAB and seeing this.

pacak commented 11 months ago

The problem is that you use completion files generated by an older version, at some point decided to break compatibility to make things easier. I'll look into how other apps are handling it I guess.

pacak commented 10 months ago

Would it be better if I replace the panic with a print to stderr + exit with error code 0? This way user will still get a notification about required changes on their side but it will look something like this instead

myapp --foo<TAB>
Unsupported output revision 4, you need to genenerate your shell completion files for the app
pacak commented 7 months ago

Current behavior looks like this:


% cargo-asm Unsupported output revision 17, you need to genenerate your shell completion files for the app                                                                                                                        ```

Do you still think silent failure is preferable?
zmitchell commented 7 months ago

I still think silent failure is preferable. Maybe emit a debug-level log message so that a developer can debug the situation. An end user that gets their system into a weird state such that autocomplete no longer works is more likely to be annoyed by seeing these messages rather than autocomplete mysteriously not working.

pacak commented 7 months ago

Okay. I'll make it a silent failure in the release mode and something with a message in dev mode.

pacak commented 5 months ago

Now it panics in dev mode but simply exits without producing anything in release mode.