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

add new example: stopwatch #503

Merged
merged 35 commits into from
Oct 9, 2020
Merged

add new example: stopwatch #503

merged 35 commits into from
Oct 9, 2020

Conversation

TianyiShi2001
Copy link
Contributor

@TianyiShi2001 TianyiShi2001 commented Sep 21, 2020

A stopwatch is implemented in this example.

This example is not complete. I managed to make the textbox update the elapsed time every second, but haven't figured out how I can pause or stop the stopwatch. @gyscos any suggestions?

@gyscos
Copy link
Owner

gyscos commented Sep 22, 2020

One solution is not to use a thread at all; instead, a structure will hold:

  • The current status (paused or running)
  • The current elapsed time
  • The time of last_update.

Then:

  • To read() the time: if status is paused, return elapsed. Otherwise, return elapsed + now - last_update.
  • To pause() the timer, update elapsed with the formula above, update last_update, then change the status.
  • To resume() the timer, update last_update and change the status.

This struct (probably in a Rc<RefCell>, or Arc<Mutex>) can be used to implement a custom view. Cursive::set_fps(5) or so can be used to automatically re-draw the view.

You could also do it using a thread; you would pause the watch by stopping the thread (for example setting a running: Arc<AtomicBool> to false, or using channels), and "resume" by spinning a new thread. Care should be taken not to "resume" twice and so on.

@TianyiShi2001
Copy link
Contributor Author

Thank you for the help! I've now made the timer to start/pause/resume by responding to pressing "Space". It's the first time I implement a View so I ran into some problems.

I also want to implement stop and on_stop which mirror SelectView's submit and on_submit. However, if I write

       fn stop(&mut self) -> EventResult {
            self.pause();
            let cb = self.on_stop.clone().unwrap();
            // We return a Callback Rc<|s| cb(s, &*v)>
            EventResult::Consumed(Some(Callback::from_fn(move |s| {
                cb(s, self.elapsed)
            })))
        }

I get the error: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement. How can I get around this?

@gyscos
Copy link
Owner

gyscos commented Sep 22, 2020

I think when you write self.elapsed in the move closure, it tries to move self.
Instead, define let elapsed = self.elapsed; before, since it's Copy it should let the closure move just the elapsed time, leaving self alone.

Also note: EventResult::with_cb is basically a shortcut for EventResult::Consumed(Some(Callback::from_fn(...))).

@TianyiShi2001
Copy link
Contributor Author

I think when you write self.elapsed in the move closure, it tries to move self.
Instead, define let elapsed = self.elapsed; before, since it's Copy it should let the closure move just the elapsed time, leaving self alone.

Exactly! Thank you!

@TianyiShi2001
Copy link
Contributor Author

Adding the 'lap time' functionality might make this example too complicated. Instead, I am going to implement a fully-fledged TUI clock utility in a separate crate.

}
let result = if self.on_stop.is_some() {
let cb = self.on_stop.clone().unwrap();
let stopwatch_data = self.stopwatch.clone(); // TODO: remove clone
Copy link
Contributor Author

@TianyiShi2001 TianyiShi2001 Sep 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost done... Except in line 163 where I haven't figured out how not to clone

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeaah... the issue is that the callback will be called later, possibly after removing this view. So we can't depend on any data in the view itself; the callback must be self-supported.
The solution is to either clone the data as you are doing (making it standalone), or wrap it in a Rc shared by both the view and the callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just came up with another approach: use mem::replace to replace self.stopwatch with a new, zeroed StopWatch, while getting the ownership of the 'freshly' produced data, which can then passed to the closure.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use std::mem::take which does this automatically for "default" types.

@TianyiShi2001 TianyiShi2001 marked this pull request as ready for review September 23, 2020 06:13
@TianyiShi2001
Copy link
Contributor Author

I realized that EventResult::with_cb cannot act as the shortcut for EventResult::Consumed(Some(Callback::from_fn_once(...))) and I have to use the latter form.

Now, this example pretty much works as I expected! Thank you so much for the guidance!

@gyscos
Copy link
Owner

gyscos commented Sep 23, 2020

Thanks!
A dedicated crate for more features sounds like a great idea. This way we can simplify the example to the minimum, and it'll be a teaser for your library :p

@TianyiShi2001
Copy link
Contributor Author

Thank you for all the help! I've simplified the example. Meanwhile, I've implemented the basic countdown timer in my crate.

@gyscos
Copy link
Owner

gyscos commented Oct 8, 2020

I think we can simplify a bit further the example - let's focus on one feature (a stopwatch) and look for the minimal example demonstrating this.

For example: https://gist.github.com/gyscos/67910f22e304214eaef00157faecb2ce

@TianyiShi2001
Copy link
Contributor Author

Cool, I've adopted your changes. Thank you!

@gyscos
Copy link
Owner

gyscos commented Oct 9, 2020

Great! I think you just need to rebase to resolve the readme conflict.

TianyiShi2001 and others added 6 commits October 9, 2020 03:11
Now the `OnEventView.on_pre_event_inner()` calls return
`Some(EventResult::Consumed(Some(Callback)))` instead of
`Some(EventResult::Consumed(None))`.
This follows the guidelines from documentation of methods returning a
`Callback`, which say that it should be ran on the `Cursive`.
While in this example this doesn't make a difference, the previous
version created confusion for new users who might not realize you can
pass the `Callback`s to the `Cursive` this way.
@TianyiShi2001
Copy link
Contributor Author

Looks good now

@gyscos gyscos merged commit c7c5f79 into gyscos:main Oct 9, 2020
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.

3 participants