Skip to content

Conversation

ryan-summers
Copy link
Member

This PR refactors the Runner to take ownership of the root menu. This enables the menu to be created on an ad-hoc basis (i.e. not at the crate root) to be handed to the Runner without lifetime worries.

The impact is that we can't keep references to the menu internally, so we have to traverse the menu throughout operation. Open to some suggestions.

For an example as to why this is needed, refer to https://github.com/quartiq/stabilizer/pull/813/files#diff-f0b20facc8d3d2dfccda7199e70cb8af5ba9255d80e1d35fdd950dc6a2d567a3R30

I'm trying to build a generic "Serial Console Settings Manager" that uses menu for providing the UI for settings management

@thejpster
Copy link
Member

thejpster commented Nov 15, 2023

The Runner object takes a &Menu so that the Menu object can live in .rodata in an immutable static. This change will mean the menu gets copied on to the stack, and could potentially use 100s of bytes (I'm guessing - I haven't checked).

Maybe we could change the runner to take AsRef<Menu>? Then it will work with either Menu or &Menu, like those Standard Library APIs that take AsRef<str> or AsRef<Path>?

@thejpster
Copy link
Member

Wait, no. I checked and my OS_MENU is only 24 bytes. Then I remembered that the menu items live in their own slice, so the Menu is just two Option<function pointers>, a &[Item] and a &str. 6 words, so 24 bytes. All checks out.

So this PR seems fine then.

@thejpster
Copy link
Member

I think struct Menu will need to be clone, otherwise if it's a static you can't get it into the runner.

You can't derive(Clone) because rustc things that the <T> also needs to be cloneable, even though th Menu doesn't actually hold a T. So we just implement the trait manually.
@thejpster
Copy link
Member

Added a impl Clone for Menu

@thejpster
Copy link
Member

Sheesh, I haven't looked at this in ages. Have tidied up a bit.

@thejpster thejpster added this pull request to the merge queue Nov 15, 2023
Merged via the queue into master with commit bcc2488 Nov 15, 2023
@ryan-summers ryan-summers deleted the feature/owned-menu branch November 16, 2023 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants