Skip to content

Conversation

@gbourant
Copy link

@gbourant gbourant commented Mar 17, 2025

ref: #687

@gbourant
Copy link
Author

By adding < /dev/tty we make it work over pipes but it won't echo the prompt correctly if it contains an array.

    const STEPS  = ["STEP_1", "STEP_2", "STEP_3", "STEP_4"]
    const STEP    = input_prompt("Select a step: [{STEPS}]")

With my change it prints Select a step: [STEP_1

@gbourant
Copy link
Author

Well, this has nothing to do with the < /dev/tty. It has to do how Amber transpils the code to bash.

__AMBER_ARRAY_3=("STEP_1" "STEP_2" "STEP_3" "STEP_4")
local STEPS=("${__AMBER_ARRAY_3[@]}")
input_prompt__162_v0 "Select a step: ${STEPS[@]}"

If I change local STEPS=("${__AMBER_ARRAY_3[@]}") to local STEPS=("${__AMBER_ARRAY_3[*]}") (with an *) it will print as expected. So maybe when input_prompt ("Select a step: [{STEPS}]") contains an array (STEPS), the * should be used instead of @.

@Mte90
Copy link
Member

Mte90 commented Mar 19, 2025

We need to see if #678 fix that compilation, @Ph0enixKM what do you think?

@Mte90 Mte90 requested a review from Ph0enixKM March 19, 2025 12:55
@b1ek b1ek changed the title read user input when script is passed through a pipe. closes #687 read user input when script is passed through a pipe Mar 20, 2025
@Ph0enixKM
Copy link
Member

@gbourant could you provide tests for this example?

@gbourant
Copy link
Author

@Ph0enixKM, I'll check the comments and add tests this or next week.

@Ph0enixKM
Copy link
Member

@gbourant sure thing. Take your time

@gbourant
Copy link
Author

I do think that the suggestion to move the < /dev/tty outside of the quotes is correct.

Also, using read -p "$prompt" < /dev/tty forces input to come directly from the terminal, bypassing the stdin redirection (e.g., exec 0</tmp/test_input), which breaks automated testing that relies on feeding input via stdin.

If you have any suggestion let me know.

input_prompt__94_v0() {
    local prompt=$1
    read -p "$prompt" < /dev/tty
    __AS=$?
    __AF_input_prompt94_v0="$REPLY"
    return 0
}
echo "Amber" >/tmp/test_input
__AS=$?
exec 0</tmp/test_input
__AS=$?
input_prompt__94_v0 "Please enter your name:"
__AF_input_prompt94_v0__5_12="${__AF_input_prompt94_v0}"
__0_name="${__AF_input_prompt94_v0__5_12}"
echo "Hello, ""${__0_name}"
rm /tmp/test_input
__AS=$?

b1ek

This comment was marked as off-topic.

@b1ek
Copy link
Contributor

b1ek commented May 10, 2025

@Ph0enixKM we need u to approve to merge

@Ph0enixKM
Copy link
Member

Right, sorry. I took my time too.

@b1ek
Copy link
Contributor

b1ek commented May 12, 2025

@gbourant the checks are currently failing, you need to fix that before merging

@gbourant
Copy link
Author

Hello @b1ek ,

I have noticed that that's why I made the following comment.

If anyone has any suggestion, it's highly welcomed :-)

Maybe we need to use something like (it's not working)

pub fn eval_bash_pipe<T: Into<String>>(code: T) -> (String, String) {

    let mut cmd = Command::new("bash");
    cmd.stdin(Stdio::piped());
    cmd.stdout(Stdio::piped());
    cmd.stderr(Stdio::piped());

    let mut child = cmd.spawn().unwrap();
    let code_str = code.into();

    // if let Some(stdin) = child.stdin.as_mut() {
        // stdin.write_all(code_str.as_bytes()).unwrap();
    // }

    child.stdin.as_mut().unwrap().write_all(code_str.as_bytes()).unwrap();

    let output = child.wait_with_output().unwrap();

    (
        String::from_utf8(output.stdout).unwrap().trim_end().into(),
        String::from_utf8(output.stderr).unwrap().trim_end().into(),
    )
}

and

#[test]
fn pipe_test() {
    let hello = fs::read_to_string("src/tests/stdlib/no_output/pipe.ab")
    .expect("Failed to read the script file");
    let hello = compile_code(hello);
    let (stdout, _) = eval_bash_pipe(hello);
    assert_eq!(stdout, "Hello, Amber");
}
import * from "std/env"

trust $ echo "Amber" >> /tmp/test_input $
trust $ exec 0< /tmp/test_input $
let name = input_prompt("Please enter your name:")
echo "Hello, " + name
trust $ rm /tmp/test_input $

// echo "Hello, Amber"

Also note, I'm not familiar with Rust :-)

@Mte90
Copy link
Member

Mte90 commented May 19, 2025

I think that in the meantime you can improve the test for input_prompt that is in amber and for the rust stuff we can see later (there is another PR that fixes some stuff with the CI awaiting) @gbourant

@Ph0enixKM
Copy link
Member

@gbourant can you share updates on this PR?

@gbourant
Copy link
Author

gbourant commented Jul 9, 2025

@Ph0enixKM, I cannot find a way to make the test to pass. It expects the input from the TTY but it reads from the file exec 0< /tmp/test_input. As far as I did my research, there isn't any built-in way to make the test read predefined data from the TTY. Maybe we need to run those test differently like I said at #688 (comment).

Also a small note, my original PR changed only the input_prompt but other input_ methods exist.

I won’t be continuing this, but anyone who wants to pick it up is more than welcome!

pub fun input_prompt(prompt: Text): Text {
    trust $ read -p "\${nameof prompt}" < /dev/tty $
    return trust $ echo \$REPLY $
}

import * from "std/env"

// Output
// Hello, Amber

main {
    trust $ echo "Amber" >> /tmp/test_input $
    trust $ exec 0< /tmp/test_input $
    let name = input_prompt("Please enter your name:")
    echo "Hello, " + name
    trust $ rm /tmp/test_input $
}

@gbourant gbourant reopened this Jul 10, 2025
@gbourant
Copy link
Author

ok, locally this change works. check it when you can @Ph0enixKM

@b1ek b1ek requested a review from Ph0enixKM July 15, 2025 21:45
@Ph0enixKM
Copy link
Member

I'll check this PR when I come back from holidays

@Mte90
Copy link
Member

Mte90 commented Sep 22, 2025

@Ph0enixKM can you check this one?

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.

4 participants