Cannot get past "Are you sure you want to drop the database" prompt

[xpe]

docker pull rust
docker run -it rust
cargo install sqx-cli
DATABASE_URL=postgres://foo@localhost/bar_test sqlx database drop

Are you sure you want to drop the database at postgres://foo@localhost/bar_test? [n/Y]

I press Y and then get, on a new line:

Are you sure you want to drop the database at postgres://foo@localhost/bar_test? [Y/n]

(Note the Y/n instead of n/Y)

I press Y and then get the previous. Repeat ad-infinitum.

I eventually press CTRL-C to bail. I get back to the terminal, but I've lost my cursor.

[abonander]

You need to hit Enter.

[jplatte]

FWIW sqlx-cli is the only CLI I've seen that guards dangerous operation this way. Especially y being rejected on the second prompt is really confusing. I think I'd prefer "type all-uppercase YES to confirm" which is much harder to get into muscle memory than y, Enter, Enter.

[xpe]

@abonander The current behavior is confusing. Do you agree?

There are known UX patterns for handling user confirmations before dangerous operations. The current way does not fit any of those. Do you agree?

If you disagree, I can explain.

[abonander]

The behavior I wanted was what apt-get does, that is, just to print the prompt and then read a line from stdin:

Are you sure you want to drop the database at <DATABASE_URL>? (y/N):  

So you'd just type y and press Return:

Are you sure you want to drop the database at <DATABASE_URL>? (y/N):  y⏎ 

The capital N is showing what happens if you press Return without entering anything.

I agree that the current implementation tries a little too hard, and it's confusing that hitting y just appears to change the default. But the previously implemented behavior was to immediately continue as soon as you pressed y which I felt was worse: console-rs/dialoguer#66

The CLI uses the dialoguer crate for prompts so it's probably worth discussing this on their issue tracker.

[xpe]

@abonander I'm going to return to my original questions...

Should I interpret your answer as two "disagree"s?

The current behavior has these characteristics:
(1) No user keypresses are printed to the screen.
(2) If a user presses 'n', 'N', 'y', or 'N', then the prompt is printed again.
(3) The cursor disappears

I just want to check if we are seeing the same behavior. Once we're on the same page here, we can figure out how to fix it. I'll follow the trail wherever it leads. I appreciate your work on this crate. If it is a problem with dialoguer, I'll chase it down.

All of this said, I have a hunch that this crate's use of async for handling CLI I/O is a contributing factor to this problem.

[abonander]

sqlx-cli does not use async I/O for stdin/stdout.

I agree that it should show the user response and not clear and re-print the prompt. That's what I meant by "trying too hard".

The confirmation is done via the dialoguer crate here: https://github.com/launchbadge/sqlx/blob/master/sqlx-cli/src/database.rs#L17-L24

So this is an issue with the behavior of dialoguer::Confirm.

[xpe]

sqlx-cli does not use async I/O for stdin/stdout.

To clarify, let's go back to my statement "... I have a hunch that this crate's use of async for handling CLI I/O is a contributing factor to this problem." Here is what I was referring to:

async fn main() {

A program doesn't have to start the Tokyo runtime with main. My hunch was using Rust's async capabilities might have been affecting the CLI I/O (by which I mean user input/output on STDIN).

An alternative way to setup the sqlx CLI app would be to have a non-async fn main() and then start the runtime after the user interaction.

Back to the main topic: All of this said, I created a dialoguer sample app and found that two out of three of these happen:

(1) User keypresses are not printed to the screen.
(2) If a user presses 'n', 'N', 'y', or 'N', then the prompt is printed again.
(3) The cursor disappears

[abonander]

You're missing the .wait_for_newline(true).default(false) options in that sample app.

It is not an async I/O issue which you can see as there is no .await call in the method chain that invokes dialoguer in the sqlx-cli source I linked in my previous comment. You can still use blocking I/O in an async fn; though it is discouraged because it blocks a Tokio thread, in this case it doesn't really matter as we don't care about blocking concurrent tasks in a short-running program.

[xpe]

You're missing the .wait_for_newline(true).default(false) options in that sample app.

This was intentional, because I wanted a minimal example. This confirms that the issues I'm seeing in sqlx come from dialoguer.

It is not an async I/O issue which you can see as there is no .await call in the method chain that invokes dialoguer in the sqlx-cli source I linked in my previous comment.

#[tokio::main]
async fn main() {
    let matches = Opt::into_app().version(crate_version!()).get_matches();

    // no special handling here
    if let Err(error) = sqlx_cli::run(Opt::from_arg_matches(&matches)).await {
        println!("{} {}", style("error:").bold().red(), error);
        std::process::exit(1);
    }
}

^^^ There is an await right there.

[abonander]

Yes, but in the specific context where dialoguer is invoked there is no .await, so execution will not yield at that point; it uses blocking I/O internally and returns the value synchronously. Thus, not an async I/O issue.

[xpe]

@abonander Have you read https://journal.stuffwithstuff.com/2015/02/01/what-color-is-your-function/ ? Also, do you understand why I'm asking this question?

[xpe]

Also, I'm not sure if you followed me when I was saying this:

An alternative way to setup the sqlx CLI app would be to have a non-async fn main() and then start the runtime after the user interaction.

[xpe]

Thus, not an async I/O issue.

To clarify, I never said it was "an async I/O" issue.

Here is what I said:

My hunch was using Rust's async capabilities might have been affecting the CLI I/O (by which I mean user input/output on STDIN).

Do you understand why I'm making this clarification?

[abonander]

Tell me what you think is happening and why lifting it out of an async fn would help.

[xpe]

I've tried very hard to ask questions -- to see if we are on the same page. I respect your work on this project, but you have avoided answering many of my questions, quite consistently. This is not a good way to communicate. I'm not happy with this dynamic, and I'm not interested in discussions that continue in such a fashion.

[abonander]

Okay, let me answer your questions then.

The current behavior is confusing. Do you agree?

Yes.

The current way does not fit any of those. Do you agree?

Yes.

Have you read https://journal.stuffwithstuff.com/2015/02/01/what-color-is-your-function/ ?

Yes.

Also, do you understand why I'm asking this question?

No, please elaborate.

Do you understand why I'm making this clarification?

No, please elaborate.

[mehcode]

This conversation has went off the rails a bit. I'm going to close and lock this with a short summary:

• The current behavior is not ideal. Everyone in this thread, @abonander included, seem to agree that this is correct. We are deferring to a crate here, maybe the answer is we need to either do the prompt ourselves or use a different crate.

• Async IO has nothing to do with how the prompt works.

@xpe This is not a direct priority for me however. If you would like to assist us here, we would certainly appreciate a PR that improves the prompt to how apt-get behaves, which was the original intention.