Implement transaction countdown timer using ANSI escape codes #333
Conversation
| const REFRESH_INTERVAL: u64 = 100; | ||
|
|
||
| pub struct Timer { | ||
| bus: Bus<String> |
There was a problem hiding this comment.
I think we should try do this without a new dependency. Each new dependency needs to carry its weight. So, we're gaining a new feature but is it that much better than what the stdlib or tokio offer? For example, tokio already has synchronization primitives (https://docs.rs/tokio/0.2.3/tokio/sync/index.html) that might be good enough here.
Adding dependencies is not evil, but they do increase compile times (more to link, fresh builds are slower) and require maintenance as new versions come out or there are security issues (maybe in them, or what they drag in too).
There was a problem hiding this comment.
I agree with the concern about adding new dependencies. However, bus provides a zero buffer broadcast function that is unavailable in tokio (the most similar function would be watch, but this stores the latest value, which results in a bug when stop_timer is called and there are zero subscribers to the bus, then start_timer is called)
There was a problem hiding this comment.
What about notify? Or, if we use a task, we could cancel it?
|
|
||
| pub fn run_timer(&mut self) { | ||
| let mut recv = self.bus.add_rx(); | ||
| thread::spawn(move || { |
There was a problem hiding this comment.
I'm not so sure about the thread here. Is there a way to do this with async instead, such that we have concurrency but not parallelism? I think the way you wrote this is race-safe (given the move to top left + restore), but it'd probably make more sense to have this be part of the overall IO loop. This probably comes back to your question - can we do this with rustyline or switch to some other library that has animations/feedback as a feature?
There was a problem hiding this comment.
Currently, rustyline does not provide an async interface. I'm currently looking into implementing this, however.
There was a problem hiding this comment.
Ideally we'd simply store the start time of the transaction in a variable somewhere and then define a render function that computed the remaining time based on now-start.
I'm not sure that the linked issue is relevant here. The missing piece is some sort of render hook.
There was a problem hiding this comment.
What should be responsible for the render function? Could you clarify on including this render process in to overall IO loop? Would the ideal solution not spawn any threads?
| use bus::Bus; | ||
| use std::io::{self, Write}; | ||
|
|
||
| const SAVE: &str = "\x1b7"; |
There was a problem hiding this comment.
We probably need something like termcap to figure out if we're able to do this at all. Otherwise we might just wreck the user's console on cmd.exe or similar.
|
What happens when the screen is already full? Does the timer overwrite previous input/output? |
|
Yes- the current implementation uses the erase line ANSI Escape Code |
|
It seems to me the thing we're trying to warn customers about is that the query they're about to input could fail. And so the UI feedback should be close to the input. Having it far away (worse case, top left while the cursor is bottom right) may make it hard to spot or intuit what it means. And, having it override previous results is annoying. One use-case for the CLI is to do a bunch of queries and then copy the interaction to something (like a github issue). So I think this needs to be unobtrusive as a design requirement. What are your thoughts? |
|
I agree. Ideally, the * in the prompt should be replaceable with a countdown timer. I wonder if the timer should be tightly coupled with the transaction lifecycle, though. If the timer ends prematurely, should the transaction automatically be aborted, bringing the user back to the default blue prompt? |
That sounds reasonable.
I don't think so. |
|
I implemented a readline library which accomplishes the live timer using |
I think it's reasonable to try cram this feature in within the confines of rustyline, then as a separate item figure out if rustyline is the right longterm bet. The idea of the prompt having a "timer space" in it that we are allowed to fiddle with is pretty reasonable. It provides a decent opt-in/out experience. |
Issue #, if available:
#203
Description of changes:
This is a bit of a hacky change until I figure out how to make asynchronous calls to draw prompts using the
rustylines, or use another readline implementation altogether.This solution relies on the customer's terminal handling the
ESC7andESC8ANSI escape codes as Save Cursor Position and Restore Cursor Position, respectively. This is supported in VT100 Mode, which is apparently supported by thextermterminal emulator.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.