mcfly icon indicating copy to clipboard operation
mcfly copied to clipboard

replace termion with crossterm and make colors configurable

Open dmfay opened this issue 3 years ago • 9 comments

This started because I wanted to customize colors, and turned into something of a rabbit hole. To summarize:

  • dynamically setting colors in termion is awkward (cf broot)
  • the relevant feature request is two years old with minimal activity
  • in the mean time, broot has replaced termion with crossterm, which is not quite a drop-in API replacement, improves on it in the colors department, and opens up the possibility of supporting Windows since termion is *nix-only

Outside the terminal code, mcfly now reads colors and other settings from ~/.mcfly/mcfly.toml, defaulting to the original styling and config if the file doesn't exist. Environment variables are still supported and there's a self-describing sample config file. I don't have a Windows machine to test compatibility, so there are likely as not more obstacles, but it's a big step closer.

~I did remove MCFLY_LIGHT entirely, so this conflicts with #151 (sorry!). If we want multiple colorschemes, figment's switchable profiles may be useful.~

It also conflicts with #149, but that I can rebase.

dmfay avatar Jun 03 '21 21:06 dmfay

Thanks @dmfay for working on this! Since some users use MCFLY_LIGHT, how easy do you think it would be to add it for backwards compatability?

Also, please note that my wife and I just had our first child this week and so I will be slow to reply for a while...

cantino avatar Jun 05 '21 21:06 cantino

Congratulations to both of you!

Detecting MCFLY_LIGHT and supplying alternate default colors turned out to be trivial, and would clear the way for #151 too -- updated.

dmfay avatar Jun 05 '21 22:06 dmfay

Nice!

cantino avatar Jun 06 '21 00:06 cantino

Here's the rebase, for whenever's convenient! This version also prints a full-width cursor, but enough else changed over time in interface.rs that squashing was the better part of valor.

dmfay avatar Jun 06 '21 02:06 dmfay

Hey @dmfay, are you up for another rebase? I think the only change is moving ctrl-d to delete a character instead of exit from the interface.

I'm getting this error when I try to build this PR:

error[E0658]: the `#[track_caller]` attribute is an experimental feature
  --> /Users/andrew/.cargo/registry/src/github.com-1ecc6299db9ec823/figment-0.10.5/src/providers/serialized.rs:75:5
   |
75 |     #[track_caller]
   |     ^^^^^^^^^^^^^^^
   |
   = note: see issue #47809 <https://github.com/rust-lang/rust/issues/47809> for more information

error[E0658]: the `#[track_caller]` attribute is an experimental feature
  --> /Users/andrew/.cargo/registry/src/github.com-1ecc6299db9ec823/figment-0.10.5/src/providers/serialized.rs:91:5
   |
91 |     #[track_caller]
   |     ^^^^^^^^^^^^^^^
   |
   = note: see issue #47809 <https://github.com/rust-lang/rust/issues/47809> for more information

Do we have to use experimental features now?

cantino avatar Jun 20 '21 22:06 cantino

Updated! As for experimental features, you might need to update Rust? I'm not seeing that issue with 1.53 (and track_caller has made it to stable).

dmfay avatar Jun 20 '21 23:06 dmfay

I updated rust and it builds successfully! However, now I'm getting:

thread 'main' panicked at 'McFly error: failed to read input IoError(Custom { kind: Other, error: "Failed to initialize input reader" })', src/interface.rs:340:43

When I try to launch mcfly.

cantino avatar Jun 21 '21 22:06 cantino

I can replicate that with osx + zsh + ctrl-r -- in linux it's fine, in bash it's fine, running mcfly search works. Nothing jumps out at me in the zsh scripting though, and the full backtrace I cleaned up doesn't offer any obvious leads either.

thread 'main' panicked at 'McFly error: failed to read input IoError(Custom { kind: Other, error: "Failed to initialize input reader" })', src/interface.rs:340:43
stack backtrace:
0: 0x10d0ce754 - std::backtrace_rs::backtrace::libunwind::trace::h67b1783e93f517c7
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/../../backtrace/src/backtrace/libunwind.rs:90:5
1: 0x10d0ce754 - std::backtrace_rs::backtrace::trace_unsynchronized::h6642b95f229b3f12
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
2: 0x10d0ce754 - std::sys_common::backtrace::_print_fmt::h0a233062426578e9
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:67:5
3: 0x10d0ce754 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h5b5b7de1c79b4b45
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:46:22
4: 0x10d0f04ce - core::fmt::write::h65ec8c8a6aa7b549
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/fmt/mod.rs:1094:17
5: 0x10d0cb1da - std::io::Write::write_fmt::hc52e560ab9e57db4
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/io/mod.rs:1584:153c2-1590682
6: 0x10d0d058f - std::sys_common::backtrace::_print::h27e4664250aa8eaf
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:49:5
7: 0x10d0d058f - std::sys_common::backtrace::print::hb149f667a3c579ca
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:36:9
8: 0x10d0d058f - std::panicking::default_hook::{{closure}}::h0974624c30a6e2b5
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:208:50
9: 0x10d0d0099 - std::panicking::default_hook::hda3cfa32e0773aac
     at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:225:9
10: 0x10d0d0d55 - std::panicking::rust_panic_with_hook::h1cdaa4b48d8ea4a1
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:591:17
11: 0x10d0d0865 - std::panicking::begin_panic_handler::{{closure}}::h51157b4dbc4c5d91
   at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:497:13
12: 0x10d0cebe8 - std::sys_common::backtrace::__rust_end_short_backtrace::h8f7fee8b4c5a0c05
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:141:18
13: 0x10d0d07ca - rust_begin_unwind
     at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:493:5
14: 0x10d0d077b - std::panicking::begin_panic_fmt::h2d14b6e3309decb9
  at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:435:5
15: 0x10cb1e3fd - mcfly::interface::Interface::select::{{closure}}::h90d8e7250ab1ab3c
    at /Users/dian/work/mcfly/src/interface.rs:340:43
16: 0x10ca507af - core::result::Result<T,E>::unwrap_or_else::h9fb4708dbefaa746
    at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/result.rs:821:23
17: 0x10cb1dd3d - mcfly::interface::Interface::select::h42cf7b2130881907
    at /Users/dian/work/mcfly/src/interface.rs:340:17
18: 0x10cb18c03 - mcfly::interface::Interface::display::h5541174453fd958a
     at /Users/dian/work/mcfly/src/interface.rs:111:9
19: 0x10ca0d907 - mcfly::handle_search::h408c96b2c1212afb
       at /Users/dian/work/mcfly/src/main.rs:46:18
20: 0x10ca0e325 - mcfly::main::h8d16aec9d2e01a04
  at /Users/dian/work/mcfly/src/main.rs:107:13
21: 0x10ca0e55e - core::ops::function::FnOnce::call_once::h0f631598e64ddf41
      at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/ops/function.rs:227:5
22: 0x10ca0edd1 - std::sys_common::backtrace::__rust_begin_short_backtrace::hec7b2049f2174e7a
  at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:125:18
23: 0x10ca0ebc4 - std::rt::lang_start::{{closure}}::h44dd4ec2cae74050
     at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/rt.rs:49:18
24: 0x10d0d10c1 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hc43f50f6c262e503
     at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/ops/function.rs:259:13
25: 0x10d0d10c1 - std::panicking::try::do_call::ha2a21b2645967438
    at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:379:40
26: 0x10d0d10c1 - std::panicking::try::h1ca18252e93fb057
  at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:343:19
27: 0x10d0d10c1 - std::panic::catch_unwind::h64441306bfaea78f
    at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panic.rs:431:14
28: 0x10d0d10c1 - std::rt::lang_start_internal::h37583e1fa93097c4
       at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/rt.rs:34:21
29: 0x10ca0eb9e - std::rt::lang_start::h94acaba22ce781a2
  at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/rt.rs:48:5
30: 0x10ca0e3b6 - _main

https://github.com/crossterm-rs/crossterm/issues/500 may be relevant -- here's another crossterm-using project with an osx-only problem. I disabled the terminfo changes and tty pipe in mcfly-history-widget and that didn't avert a panic, though.

dmfay avatar Jun 22 '21 00:06 dmfay

Hey, I just want to say I'd love to have configurable colors, this looks broken in Unikitty Light as well. 👍 I tried your branch @dmfay and I get the same results.

bahlo avatar Jul 21 '21 09:07 bahlo

👋🏼 Another Crossterm user here, that just happened to have one of my projects linked in a comment. Crossterm just pushed a fix for https://github.com/crossterm-rs/crossterm/issues/500, so it's possible that the problem blocking this PR has been addressed.

I haven't tried the fix in my project yet, but figured a ping here wouldn't hurt.

MitMaro avatar Jan 11 '23 19:01 MitMaro