goodell / radix-calc

a CLI programmer's calculator written in Rust, with an Alfred workflow GUI
MIT License
16 stars 3 forks source link

Panic for valid 64-bit number #2

Open atkinsam opened 11 months ago

atkinsam commented 11 months ago

The script will panic when large 64-bit values are used. A few examples:

0xffffffffffffffff
0xfffffffc0b172d00

Log output from Alfred:

[14:43:14.641] radix-calc[Script Filter] Queuing argument '0xffffffffffffffff'
[14:43:14.680] radix-calc[Script Filter] Script with argv '(null)' finished
[14:43:14.688] ERROR: radix-calc[Script Filter] Code 101: thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: Overflow }', src/libcore/result.rs:859
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: radix_calc::radix_calc::__parse_num_lit
  10: radix_calc::radix_calc::__parse_atom
  11: radix_calc::radix_calc::__parse_infix_arith::__infix_parse
  12: radix_calc::radix_calc::expr
  13: radix_calc::main
  14: std::panicking::try::do_call
  15: __rust_maybe_catch_panic
  16: std::rt::lang_start
  17: start
goodell commented 11 months ago

Thanks for the nice bug report. I won’t have the right computer access to fix it for a couple of weeks. If you want to try a patch I’d happily take a fix PR that includes a regression test.

atkinsam commented 11 months ago

I ended up working around this using Rust's i128 type. I'm sure there are nicer ways to be smart about precision, let the user choose, etc. but I didn't have a chance to try that out. Let me know what you think. If you're good with it, I'll create the PR.

https://github.com/goodell/radix-calc/compare/master...atkinsam:radix-calc:fix-overflow-i64

There are some other small unrelated changes in that branch. I disable the "automatically paste to frontmost app" option in Alfred, so the copy+paste message in the option subtitle didn't make sense for my workflow.