rustne-kretser / noline

IO-agnostic line editor for embedded systems
Mozilla Public License 2.0
96 stars 9 forks source link

Unexpected ParserError #12

Open Tim-Paik opened 2 years ago

Tim-Paik commented 2 years ago

I'm having some issues trying to use this crate to make an interaction for my no_std project. I tried implementing my own IO type (for my environment), but always panic when building the editor:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParserError', src/main.rs:9:10
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:142:14
   2: core::result::unwrap_failed
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/result.rs:1814:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/result.rs:1107:23
   4: testkey::main
             at ./src/main.rs:6:22
   5: core::ops::function::FnOnce::call_once
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5

here is my code:

// main.rs
use noline::builder::EditorBuilder;
use std::io::Write;

fn main() {
    let mut io = Console::new();
    let mut editor = EditorBuilder::new_unbounded()
        .with_unbounded_history()
        .build_sync(&mut io)
        .unwrap();
    loop {
        if let Ok(line) = editor.readline("> ", &mut io) {
            write!(io, "Read: '{}'\n\r", line).unwrap();
        } else {
            break;
        }
    }
}

pub struct Console(Vec<u8>);

impl Console {
    pub fn new() -> Self {
        let vec = Vec::from([b'a', b'b', b'c']);
        Self(vec)
    }
}

impl noline::sync::Read for Console {
    type Error = core::convert::Infallible;

    fn read(&mut self) -> Result<u8, Self::Error> {
        if let Some(byte) = self.0.last() {
            return Ok(*byte);
        } else {
            panic!("should panic here");
        }
    }
}

impl noline::sync::Write for Console {
    type Error = std::io::Error;

    fn write(&mut self, word: &[u8]) -> Result<(), Self::Error> {
        std::io::Write::write(&mut std::io::stdout(), word).map(|_| ())
    }

    fn flush(&mut self) -> Result<(), Self::Error> {
        Ok(())
    }
}

impl Write for Console {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        std::io::Write::write(&mut std::io::stdout(), buf)
    }

    fn flush(&mut self) -> std::io::Result<()> {
        std::io::stdout().flush()
    }
}

Did I write something wrong? It looks like everything works fine

dimpolo commented 1 year ago

I had a similar problem so I went digging:

EditorBuilder::build_sync will send these bytes and expect a response indicating the terminal size https://github.com/rustne-kretser/noline/blob/6e6d8b79a0cb79501ffaccbf6b2a81eab17ae46c/noline/src/core.rs#L46-L52

Your Console provides an invalid response which results in the error.

Btw, you should use self.0.pop() instead of self.0.last() in your Read impl.

eivindbergem commented 1 year ago

@Tim-Paik Sorry for not responding, I missed the github notification so I didn't see this before now.

Do you have a full example I can have a look at?

Tim-Paik commented 1 year ago

@Tim-Paik Sorry for not responding, I missed the github notification so I didn't see this before now.

Do you have a full example I can have a look at?

You can execute the following command to reproduce my problem

cargo new nolinetest
cd nolinetest
cargo add noline --features alloc

Populate src/main.rs with the code from main.rs that I provided at the beginning, after that, you can use cargo run to run it

I had a similar problem so I went digging:

EditorBuilder::build_sync will send these bytes and expect a response indicating the terminal size

https://github.com/rustne-kretser/noline/blob/6e6d8b79a0cb79501ffaccbf6b2a81eab17ae46c/noline/src/core.rs#L46-L52

Your Console provides an invalid response which results in the error.

Btw, you should use self.0.pop() instead of self.0.last() in your Read impl.

Looks like you're making sense, since I'm not providing a terminal width, which noline definitely needs. But I don't know how should I provide the size of the terminal...

eivindbergem commented 1 year ago

Noline expects to be connected to an ANSI terminal. During init, noline probes the terminal using ANSI escape codes to get the size of the terminal. You can have a look at here for an example of a mockterminal.

But, what is your use case here? Without an actual terminal, noline isn't of much use.

rmsc commented 1 month ago

Not entirely sure if related, but I'm consistently getting this panic on the rp2040 example. Regardless of the dumb terminal emulator I use, the first time I hit "enter" it panics at the unwrap:

    let mut editor = EditorBuilder::from_slice(&mut buffer)
        .with_slice_history(&mut history)
        .build_sync(&mut io)
        .unwrap();

I had to modify the example slightly for it to work on the second attempt:

    let mut editor = loop {
        break match EditorBuilder::from_slice(&mut buffer)
            .with_slice_history(&mut history)
            .build_sync(&mut io) {
                Ok(ed) => ed,
                Err(err) => {
                    let error = match err {
                        NolineError::IoError(_) => "IoError",
                        NolineError::ParserError => "ParseError",
                        NolineError::Aborted=> "Aborted",
                    };
                    writeln!(io, "Error: {}\r", error).unwrap();
                    continue;
                }
        }
    };

Now it works, but the error still happens:

$ picocom /dev/ttyACM1
(...)
Terminal ready
Error: ParseError
>

Is this something that's fixable on noline's side, or should the example be modified to keep retrying?

eivindbergem commented 1 month ago

I would need to be able to reproduce this in order to debug it. Could you provide more details of your setup:

rmsc commented 1 month ago

Thanks!

  • What operating system are you running?

Linux 6.10.8 (archlinux)

  • Does this happen with other terminal emulators (minicom, etc)?

Yes, I've tried minicom and screen, same thing on both.

  • What terminal settings are you using (output of stty -F /dev/ttyACM?
$ stty -F /dev/ttyACM1
speed 9600 baud; line = 0;
min = 1; time = 0;
-brkint -icrnl -imaxbel
-opost
-isig -icanon -iexten -echo
  • What UART-to-USB adapter are you using?

This is a straight USB connection to the rp-pico (using rp2040 usb serial example).

eivindbergem commented 1 month ago

Thanks! I don't have an RP pico here now, but I'll try to bring one tomorrow and have look.

eivindbergem commented 1 week ago

I've done some testing, and it appears that when using minicom the serial port is opened for read/write before configuring it, which means that dtr() is true on the device, but raw -echo hasn't been set yet. Try running stty raw -echo -F /dev/ttyACM1 before starting minicom and see if that helps.

I guess noline should be a bit more robust in the face of unexpected data, but I'm not sure how this should be handled.