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

Revert "Switch to StdioInputMethod when TERM is 'dumb' (#907)" #942

Closed
wants to merge 1 commit into from

Conversation

tompng
Copy link
Member

@tompng tompng commented May 1, 2024

Reverts #907 to avoid ruby/ruby CI failure and Debug compatibility test failure

@st0012
Copy link
Member

st0012 commented May 1, 2024

@dgutov sorry that we need to revert your PR for now as it'd break Ruby CI and block other Ruby committer's work.

This is because as part of its CI suites, debug (bundled gem) will be tested against the latest ruby/ruby, which includes IRB commits synced there. And currently debug's tests rely on the TERM=dumb behaviour before you PR. But unlike IRB, debug is only synced to ruby/ruby when a release happens.

This means that to prevent Ruby CI from failing, we will need to:

  1. patch debug to adjust for your change
  2. And then cut a release with it

But unfortunately we don't have control over the debug project, so we need to coordinate with its maintainer.

@dgutov
Copy link
Contributor

dgutov commented May 1, 2024

That's too bad, but ok.

Could you please Cc me when you file the corresponding issue or PR to the debug repo?

@tompng
Copy link
Member Author

tompng commented May 3, 2024

@dgutov Thank you for fixing #907, and sorry for making confusion.

With the workaround #943, now we don't need to revert.
I've opened a pull request to debug ruby/debug#1100 that fixes the test to pass without the workaround.

@tompng tompng closed this May 3, 2024
@tompng tompng deleted the revert_907 branch May 3, 2024 04:55
@dgutov
Copy link
Contributor

dgutov commented May 3, 2024

Thanks.

The new workaround is not ideal (an extra newline is unexpected), but it's still better than Irb being entirely non-functional. Looking forward to having it reverted after all test suites are updated.

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