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

Ensure no escape sequence before printing prompt #716

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented Jun 4, 2024

Some test (readline-ext, bundler) expects output of Readline.readline("prompt>") to match /\Aprompt>/.

Readline prints "\e[?2004h" (enable bracketed-paste) before prompt if STDIN and STDOUT is both tty.

$ ruby -rreadline -e "Readline.readline 'prompt>'" | ruby -e "p STDIN.read"
"\e[?2004hprompt>a\b\e[Kb\b\e[Kc\n\e[?2004l\r"

Readline does not print escape sequence before prompt if STDIN or STDOUT is not a tty. But Reline prints it.

$ printf "a\bb\bc\n" | ruby -rreadline -e "Readline.readline 'prompt>'" | ruby -e "p STDIN.read"
"prompt>a\b\e[Kb\b\e[Kc\n"

This pull request fixes Reline to align with Readline.

Background

Ruby ci was failing. https://github.com/ruby/ruby/actions/runs/9352049059/job/25739247917
This pull request fixes the problem of Reline side. Unfortunately, test still fails. I'll also open a pull request to Rubygems to relax the test assertion.

@tompng tompng force-pushed the no_escape_sequence_before_prompt branch from 9ecfa7f to 6e006b0 Compare June 4, 2024 15:07
@k0kubun
Copy link
Member

k0kubun commented Jun 4, 2024

Now that the rubygems PR has been merged, let's use this to pass the CI without reverting the previous change. I reverted the revert now, so let me merge this.

@k0kubun k0kubun merged commit f9227b5 into ruby:master Jun 4, 2024
40 checks passed
@tompng tompng deleted the no_escape_sequence_before_prompt branch June 5, 2024 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants