reedline icon indicating copy to clipboard operation
reedline copied to clipboard

Problems with input requiring more lines than available in the terminal window screen

Open fdncred opened this issue 2 years ago • 21 comments

using engine-q, i was testing out pasting json and was presented with this panic.

this is the json i pasted.

{
  "name": "inc",
  "usage": "Increment a value or version. Optionally use the column of a table.",
  "extra_usage": "",
  "required_positional": [],
  "optional_positional": [],
  "rest_positional": {
    "name": "",
    "desc": "",
    "shape": "Any",
    "var_id": null
  },
  "named": [
    {
      "long": "help",
      "short": "h",
      "arg": null,
      "required": false,
      "desc": "Display this help message",
      "var_id": null
    },
    {
      "long": "major",
      "short": "M",
      "arg": null,
      "required": false,
      "desc": "increment the major version (eg 1.2.1 -> 2.0.0)",
      "var_id": null
    },
    {
      "long": "minor",
      "short": "m",
      "arg": null,
      "required": false,
      "desc": "increment the minor version (eg 1.2.1 -> 1.3.0)",
      "var_id": null
    },
    {
      "long": "patch",
      "short": "p",
      "arg": null,
      "required": false,
      "desc": "increment the patch version (eg 1.2.1 -> 1.2.2)",
      "var_id": null
    }
  ],
  "is_filter": false,
  "creates_scope": false,
  "category": "Default"
}

this is the backtrace

thread 'main' panicked at 'attempt to subtract with overflow', /Users/fdncred/.cargo/git/checkouts/reedline-e42026a78d91c510/c11aef2/src/engine.rs:928:60
stack backtrace:
   0: rust_begin_unwind
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/panicking.rs:101:14
   2: core::panicking::panic
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/panicking.rs:50:5
   3: reedline::engine::Reedline::wrap
             at /Users/fdncred/.cargo/git/checkouts/reedline-e42026a78d91c510/c11aef2/src/engine.rs:928:60
   4: reedline::engine::Reedline::run_edit_commands
             at /Users/fdncred/.cargo/git/checkouts/reedline-e42026a78d91c510/c11aef2/src/engine.rs:822:25
   5: reedline::engine::Reedline::handle_editor_event
             at /Users/fdncred/.cargo/git/checkouts/reedline-e42026a78d91c510/c11aef2/src/engine.rs:600:21
   6: reedline::engine::Reedline::handle_event
             at /Users/fdncred/.cargo/git/checkouts/reedline-e42026a78d91c510/c11aef2/src/engine.rs:421:13
   7: reedline::engine::Reedline::read_line_helper
             at /Users/fdncred/.cargo/git/checkouts/reedline-e42026a78d91c510/c11aef2/src/engine.rs:406:39
   8: reedline::engine::Reedline::read_line
             at /Users/fdncred/.cargo/git/checkouts/reedline-e42026a78d91c510/c11aef2/src/engine.rs:315:22
   9: engine_q::main
             at ./src/main.rs:269:25
  10: core::ops::function::FnOnce::call_once
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.```

fdncred avatar Nov 19 '21 03:11 fdncred

Was able to reproduce it in a window where the length of the content would exceed the screen height. Hardening Reedline::wrap etc. against overflows should be a high priority (don't want to know what happens in release mode with either weird u16 wrapping or worse). Getting the prompt position to work when the content will force it to scroll outside the final screen is probably not entirely trivial. We might have to comprise in this case when it comes to screen integrity after a potential resize.

sholderbach avatar Nov 19 '21 20:11 sholderbach

oh that makes sense - I was wondering if it had to do with how tall the paste was but didn't have a chance to look.

I guess we should always start a full repaint at 1,1 if we can't go any higher?

sophiajt avatar Nov 19 '21 20:11 sophiajt

Could be a workaround, would potentially overpaint old stuff. Other theoretical edge case: what happens if you have y > height lines below the cursor and would try to paint them and use the save/Restore cursor position functionality, would the cursor position get broken as the screen coordinates exceed that

sholderbach avatar Nov 19 '21 20:11 sholderbach

@jntrnr I think your fixes should avoid the panic on this one, but pasting of over sized content causes scrolling as soon as it repaints. (animated prompt is problematic here)

sholderbach avatar Dec 23 '21 19:12 sholderbach

with the new pasting logic there is no panic, but the scrolling paints the largest row many times image

elferherrera avatar Dec 26 '21 19:12 elferherrera

Yeah, there's still an issue of reedline assuming the prompt is somewhere on the screen and being able to draw it. We need to add the ability for the prompt to no longer be visible and also for part of the user's input to be hidden as well.

sophiajt avatar Dec 26 '21 19:12 sophiajt

is this related to this image

The suggestion keeps repeating the prompt

elferherrera avatar Dec 26 '21 19:12 elferherrera

ya, the hinting is not quite right. I see this too.

fdncred avatar Dec 26 '21 19:12 fdncred

@fdncred do you see it in engine-q?

elferherrera avatar Dec 26 '21 20:12 elferherrera

@fdncred do you see it in engine-q?

yes. I've typed some things like

let config = {
really
long
config
section
}

and then if I type l the hinting system triggers and the massive hint takes over and if I type le I get it again and then let I get the hint again. It makes it so hard to do anything.

fdncred avatar Dec 26 '21 20:12 fdncred

Im wondering if we should show the whole hint or only the first row when you are at the bottom

elferherrera avatar Dec 26 '21 20:12 elferherrera

I think the whole hint is fine but as you start typing, when it's different, the hint should disappear. I can't remember if our current implementation does that. also, as you're typing it shouldn't scroll up the screen with every character.

fdncred avatar Dec 26 '21 21:12 fdncred

It does scroll but what should it do if the suggestion is bigger than the terminal?

elferherrera avatar Dec 27 '21 12:12 elferherrera

I think it should scroll and let you navigate the hint, but when you type something that isn't in the hint, the hint should disappear (or choose the next correct hint) and the terminal should "unscroll" to where it was before to fit the new hint.

fdncred avatar Dec 27 '21 12:12 fdncred

My take on the multiline hints would be, that multiline entries are probably less likely to be reused than single lines and we could collapse multiline hints as an ellipsis (...) on the following line at first and if you want to see the full hint you press down arrow to see the full hint and allow completion of it. And if you really want to browse through previous multiline entries you can still use the history.

sholderbach avatar Dec 27 '21 12:12 sholderbach

not trying to be contrary but I use multiline hints frequently when working on for loops in nushell with rustyline. I actually like this functionality in rustyline. Try it in nushell and see if you like it.

> for x in 0..5 {
echo $x
}

now, when you hit up arrow, the entire multiline command comes in, fully syntax highlighted and not hint colored. you can traverse up and down through it with the up/down arrow keys and change whatever you want. one exception, if you want to add more lines you have to go to the end and delete the } and then begin typing like you would add lines normally and end with the paren, and boom it just works.

I could live with the ... functionality if there was a way to get to editing mode similar to rustyline.

fdncred avatar Dec 27 '21 12:12 fdncred

Just to be sure on the terminology: history traversal: hitting up arrow with either an empty line or a prefix for quick search to get to previously executed commands hint: fish style autosuggestion that appears while you are typing and shows the last matching command. Completable by tab (reedline atm) or right arrow (fish, zsh)

Does your workflow depend on the hints or the history traversal being smooth?

I personally tend to use history traversal for bigger commands/pipelines like your aforementioned loop and hints for the quick repetitive call i simply want to repeat like

car<hint>go run --release</hint>

and with fish/zsh you can take that hint token by token via ctrl-rightarrow or alt-rightarrow

cargo run <hint>--release</hint>

The discussion inspired me to disable hints while doing the traversal to replicate the fish/zsh behavior and avoid that there is too much going on during history traversal #215

sholderbach avatar Dec 27 '21 15:12 sholderbach

But yes we should fix what ever is broken with the hinter and a collapsed hint should probably be a configurable option

sholderbach avatar Dec 27 '21 15:12 sholderbach

if it worked identically to fish, I'd be pleased. :)

fdncred avatar Dec 27 '21 18:12 fdncred

re: this issue about pasting json - it no longer panics but pasting large items is a mess right now. if you copy the json block in the top of this issue and paste it, paste now mostly works but then if you scroll up it appears that it's pasted the text a handful of times.

https://user-images.githubusercontent.com/343840/147497863-3da20920-5485-426b-a2f6-4da07c92895f.mov

fdncred avatar Dec 27 '21 18:12 fdncred

it isnt related to being large, but to parse something close to the lower edge of the terminal. Try pasting a small json but with the prompt at the bottom and you will see that it repeats. Dont press enter after pasting

elferherrera avatar Dec 27 '21 19:12 elferherrera