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

Term::read_line_initial_text() inconsistent with Term::read_line() regarding terminating newline #112

Open
RichRandall opened this issue Jan 25, 2022 · 1 comment

Comments

@RichRandall
Copy link
Contributor

RichRandall commented Jan 25, 2022

Term::read_line() allows the delimiting newline to be printed to the terminal, even though it is omitted from the returned String. This is the same as the documented behaviour of BufRead::read_line(), which Term::read_line() ends up calling.

The implementation of Term::read_line_initial_text() is not consistent with this. In this case, the newline is not printed to the terminal.

use console::Term;

fn main() {

    let term = Term::stderr();

    println!("Enter ten lines. The final five will have initial text");
    for _ in 1..6 {
        term.read_line().unwrap();
    }

    for _ in 6..11 {
        term.read_line_initial_text("Inital text ").unwrap();
    }
}

After I run the above and enter the numbers 1 to 10, my terminal looks like this:

Enter ten lines. The final five will have initial text
1
2
3
4
5
Inital text 6Inital text 7Inital text 8Inital text 9Inital text 10$
@RichRandall
Copy link
Contributor Author

This has contributed to a bug I reported in dialoger. I think it might be cleanest to fix it here. I've raised a PR showing a possible fix.

dtolnay pushed a commit to dtolnay-contrib/console that referenced this issue Feb 4, 2022
This counts all wakes of a task that happen while the task is "currently
polled". This could be from `task::yield_now()`, from `tokio::coop`, or
any other pattern that triggers the task to wake even though it's
currently being polled.

This implementation builds upon @seanmonstar's branch console-rs#55. However,
rather than detecting self-wakes by simply looking at _whether_ the task
is being polled at all, we instead detect them by checking whether the
task's span is on the current thread's stack of entered spans. This
approach should _not_ produce self-positives when a task is woken by
another thread while it is currently entered (as discussed in
tokio-rs/console#55 (comment)).

Testing this with the "burn" subcommand for the example app, which
spawns one task that does a lot of self-wakes via `task::yield`,
indicates that it is, in fact, working properly. Here are the stats for the
`burn` task:
![image](https://user-images.githubusercontent.com/2796466/132136922-2d0d9fbd-e8eb-475c-8ab8-855f8fce5127.png)
Note the very large number of self-wakes.

In comparison, here are the task stats for a "normal" task, which does
not wake itself (and is instead woken by timers):
![image](https://user-images.githubusercontent.com/2796466/132136964-514d7560-074a-41b1-9d6b-621c0dfac935.png)
Note that no self-wakes stat is displayed.

Closes console-rs#55.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Sean McArthur <sean@seanmonstar.com>
dtolnay pushed a commit to dtolnay-contrib/console that referenced this issue Feb 4, 2022
Depends on console-rs#112

This branch builds on console-rs#112 by calculating the *percentage* of a task's
wakeups that were self-wakes, and displaying them in the tasks view. It
also adds the beginnings of a rudimentary "Warnings" system, and
displays warnings for any tasks who have woken themselves for more than
50% of their wakeups. There is a new `Warn` trait for generating
warnings that can be used to add new warnings in the future.

The warnings functionality can almost certainly be expanded --- for
example, it would be nice to be able to quickly jump to the details view
for a task that has warnings. In the future, I could imagine generating web
documentation with details about a particular warning and how to fix it,
like [clippy has][1], and linking to them from the console. But, this is a start,
at least!

[1]: https://rust-lang.github.io/rust-clippy/v0.0.174/
![image](https://user-images.githubusercontent.com/2796466/132146062-9599ba12-cb17-48f8-b15c-4ba91947e282.png)
![image](https://user-images.githubusercontent.com/2796466/132146081-6dac231c-e929-4f93-b6ef-ac16f1dd166d.png)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
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

No branches or pull requests

1 participant