Skip to content
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

Adopt for new multi-threaded cursive capabilities #41

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

azat
Copy link

@azat azat commented Mar 31, 2024

Note: this includes #40

In 1 cursive now requires Send + Sync, and the reason for this is:

The View now requires Send + Sync, to allow accessing or moving views between threads.
This prevents using Rc/RefCell, and may require using Arc/Mutex instead.
This should eventually open the way for more multi-threaded processing of the view tree.

It is not always useful to always select some item (i.e. first by
sorting order, by the way it is not even how it works natively since
sort_by() applied after, and you need to call set_selected_item(0) for
this).

Consider csysdig, by default when you did not press anything it stick to
the first item by sort order, and only if you select something (Up/Down)
then it will stick to this item in the table.

And this behaviour is better, since if you did not focus any item you
want to look at the items sorted by the sort order.

And I also made the Home button to cancel this selection.
In [1] cursive now requires Send + Sync, and the reason for this is:

    The View now requires Send + Sync, to allow accessing or moving views between threads.
    This prevents using Rc/RefCell, and may require using Arc/Mutex instead.
    This should eventually open the way for more multi-threaded processing of the view tree.

  [1]: gyscos/cursive@f1f25b1
@correabuscar
Copy link

correabuscar commented Apr 15, 2024

This PR passes cargo build and cargo test with cursive-core = "0.3.5" and cursive = "0.20.0" and then I've tested grin (latest master) which passes too.

Would be nice to have it as version 0.15 maybe?

If so, it would allow grin to use an updated cursive and avoid this error:

   Compiling grin v5.3.0-alpha.1 (/home/user/1tmp/grin)
error[E0599]: the method `with_name` exists for struct `TableView<WorkerStats, StratumWorkerColumn>`, but its trait bounds were not satisfied
   --> /home/user/1tmp/grin/src/bin/tui/mining.rs:238:31
    |
238 |                 Dialog::around(table_view.with_name(TABLE_MINING_STATUS).min_size((50, 20)))
    |                                           ^^^^^^^^^ method cannot be called on `TableView<WorkerStats, StratumWorkerColumn>` due to unsatisfied trait bounds
    |
   ::: /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cursive_table_view-0.14.0/src/lib.rs:117:1
    |
117 | pub struct TableView<T, H> {
    | --------------------------
    | |
    | doesn't satisfy `_: Nameable`
    | doesn't satisfy `_: View`
    |
    = note: the following trait bounds were not satisfied:
            `TableView<WorkerStats, StratumWorkerColumn>: cursive::View`
            which is required by `TableView<WorkerStats, StratumWorkerColumn>: Nameable`
            `&TableView<WorkerStats, StratumWorkerColumn>: cursive::View`
            which is required by `&TableView<WorkerStats, StratumWorkerColumn>: Nameable`
            `&mut TableView<WorkerStats, StratumWorkerColumn>: cursive::View`
            which is required by `&mut TableView<WorkerStats, StratumWorkerColumn>: Nameable`
    = help: items from traits can only be used if the trait is in scope
help: the following trait is implemented but not in scope; perhaps add a `use` for it:
    |
17  + use cursive_core::view::nameable::Nameable;
    |

and an updated cursive-core to avoid this error:

   Compiling cursive_table_view v0.14.0
error[E0277]: `Rc<(dyn for<'a> Fn(&'a mut Cursive, H, std::cmp::Ordering) + 'static)>` cannot be sent between threads safely
   --> /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cursive_table_view-0.14.0/src/lib.rs:995:21
    |
995 | impl<T, H> View for TableView<T, H>
    |                     ^^^^^^^^^^^^^^^ `Rc<(dyn for<'a> Fn(&'a mut Cursive, H, std::cmp::Ordering) + 'static)>` cannot be sent between threads safely
    |
    = help: within `TableView<T, H>`, the trait `Send` is not implemented for `Rc<(dyn for<'a> Fn(&'a mut Cursive, H, std::cmp::Ordering) + 'static)>`
note: required because it appears within the type `Option<Rc<(dyn for<'a> Fn(&'a mut Cursive, H, std::cmp::Ordering) + 'static)>>`
   --> /usr/lib/rust/1.76.0/lib/rustlib/src/rust/library/core/src/option.rs:570:10
    |
570 | pub enum Option<T> {
    |          ^^^^^^
note: required because it appears within the type `TableView<T, H>`
   --> /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cursive_table_view-0.14.0/src/lib.rs:117:12
    |
117 | pub struct TableView<T, H> {
    |            ^^^^^^^^^
note: required by a bound in `cursive::View`
   --> /home/user/1tmp/cursive/cursive-core/src/view/view_trait.rs:34:33
    |
34  | pub trait View: Any + AnyView + Send {
    |                                 ^^^^ required by this bound in `View`

error[E0277]: `Rc<(dyn for<'a> Fn(&'a mut Cursive, usize, usize) + 'static)>` cannot be sent between threads safely
   --> /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cursive_table_view-0.14.0/src/lib.rs:995:21
    |
995 | impl<T, H> View for TableView<T, H>
    |                     ^^^^^^^^^^^^^^^ `Rc<(dyn for<'a> Fn(&'a mut Cursive, usize, usize) + 'static)>` cannot be sent between threads safely
    |
    = help: within `TableView<T, H>`, the trait `Send` is not implemented for `Rc<(dyn for<'a> Fn(&'a mut Cursive, usize, usize) + 'static)>`
note: required because it appears within the type `Option<Rc<(dyn for<'a> Fn(&'a mut Cursive, usize, usize) + 'static)>>`
   --> /usr/lib/rust/1.76.0/lib/rustlib/src/rust/library/core/src/option.rs:570:10
    |
570 | pub enum Option<T> {
    |          ^^^^^^
note: required because it appears within the type `TableView<T, H>`
   --> /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cursive_table_view-0.14.0/src/lib.rs:117:12
    |
117 | pub struct TableView<T, H> {
    |            ^^^^^^^^^
note: required by a bound in `cursive::View`
   --> /home/user/1tmp/cursive/cursive-core/src/view/view_trait.rs:34:33
    |
34  | pub trait View: Any + AnyView + Send {
    |                                 ^^^^ required by this bound in `View`

EDIT: Note however that I haven't tried to run the program(s) to see if it works correctly. It just builds ok, that's all I know :)

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