rust-embedded-community / menu

Command-line menu system for embedded Rust platforms.
Apache License 2.0
51 stars 11 forks source link

Noline support MVP #24

Closed eivindbergem closed 2 months ago

eivindbergem commented 2 months ago

I've done a quick implementation and got something working. There are some remaining issues that need to be figured out:

23

thejpster commented 2 months ago

I changed Runner so that buffer is not owned, but instead is an argument to input_byte.

I think this means the user of the crate is now free to fiddle with the contents of the buffer (even adding non-UTF-8 bytes). What prompted the change?

eivindbergem commented 2 months ago

I changed Runner so that buffer is not owned, but instead is an argument to input_byte.

I think this means the user of the crate is now free to fiddle with the contents of the buffer (even adding non-UTF-8 bytes). What prompted the change?

Editor already owns it own buffer, so it was the shortest path to get something working. A possible solution is to make Runner generic over the buffer and have to concrete implementations with &mut [u8] and Editor.

thejpster commented 2 months ago

Hmm. We might need a trait or something to abstract across either a raw buffer or an Editor. Or we can create some BasicEditor functionality which does what the crate currently does.

eivindbergem commented 2 months ago

I've made Runner generic over buffer, allowing either anything that implements AsMut<[u8]>, or Editor, depending on which constructor is used. No traits needed.

I've also added support for the dynamic prompt in noline. I've pointed the noline crate to the branch for the prompt support for now. I'll make a new release of noline when this is ready.

eivindbergem commented 2 months ago

I had to move the MenuManager to an InnerRunner to avoid Lifetime issues with the prompt, and pass interface as an argument to all the methods. I guess all the methods from InnerRunner could be moved into MenuManager itself, but I'll let you decide on how that is best solved.

thejpster commented 2 months ago

This looks OK to me. Thank you!

As you've re-written the prompt handling, would you mind looking at https://github.com/rust-embedded-community/menu/issues/25? I spotted this whilst testing.

thejpster commented 2 months ago

Thank you. Whilst testing I spotted that clippy was warning about unused imports - we now check clippy in CI in https://github.com/rust-embedded-community/menu/pull/26.

Would you mind rebasing and looking at the clippy output?

eivindbergem commented 2 months ago

Ok, now I've fixed everything, I hope :)

thejpster commented 2 months ago

Thank you!