New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support for FileManager in List and Window #38
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small detail about get_entry_index return value. This is not a thorough review.
src/widgets/list.rs
Outdated
// drawn at that point. | ||
fn get_entry(&self, p: Point) -> Option<Arc<Entry>> { | ||
fn get_entry_index(&self, p: Point) -> i32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function should probably return an Option<usize>
or Option<u32>
instead of an i32
. Using None
instead of -1
to indicate that there is no entry at the given point would be more idiomatic rust, I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah definitely.
List selection uses |
pub running: Cell<bool> | ||
pub bg: Cell<Color>, | ||
pub running: Cell<bool>, | ||
events: Vec<Event>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd favor internal mutability (RefCell<Vec<Event>>
) over requiring windows to be changed to mutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally I'd like to get rid of mutability and internal mutability entirely, by moving towards what I describe in the PR description.
Sorry I did not get around to this earlier |
List
selected
entry, which will be highlightedScrollEvent
home
/end
to jump to top and bottomenter
simulates a click on the entryWindow
This PR breaks
exec
up into a few pieces that can be called independently. This allows FileManager to still maintain its own event queue. In the future I'd like to put everything back behindexec
, but not at the window level.Instead I think it would be nice to have an
Application
which is capable of housing multipleWindow
s and accepts some sort of callback that allows the application to hook into the application loop. Which would allow behavior like in this FileManager PR (redox-os/orbutils#19), but without the app having to poke atWindow
in very specific ways.