Skip to content

Conversation

@arvidj
Copy link

@arvidj arvidj commented Dec 5, 2025

Fixes #6828

@kit-ty-kate
Copy link
Member

That looks alright on principle. Although i believe that the problem is more that your terminal doesn't support termio's non-canonical mode so a real fix could be to both test the TERM variable and if it's not dumb then test for non-canonical mode.

However that said, i'm wondering if we couldn't simplify all this and just use the simple mode all the time. This would match better with the typical unix input method. @rjbou @dra27 what do you think?

@arvidj
Copy link
Author

arvidj commented Dec 5, 2025

ok, maybe something like:

    if OpamStd.Sys.(not tty_out || os () = Win32 || os () = Cygwin ||
                      Lazy.force dumb_term || (Unix.tcgetattr stdin).c_icanon)
    then

?

@arvidj arvidj changed the title be less smart in [short_user_input] when [TERM=dumb] Be less smart in [OpamConsole.short_user_input] when [TERM=dumb] Dec 5, 2025
@kit-ty-kate
Copy link
Member

yeah that's what i was thinking about

@kit-ty-kate
Copy link
Member

Actually i just tested it in M-x shell and it wouldn't work. c_icanon still appears false even if the current terminal session doesn't support it

Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will do the job just fine for now and is correct anyway. We can improve it later i think (see #6830)

@kit-ty-kate kit-ty-kate force-pushed the aj/confirmations-on-dumb-terminals branch from 04ab3a4 to bf28bb9 Compare December 5, 2025 16:43
@kit-ty-kate kit-ty-kate force-pushed the aj/confirmations-on-dumb-terminals branch from bf28bb9 to 776598f Compare December 5, 2025 16:45
@kit-ty-kate kit-ty-kate changed the title Be less smart in [OpamConsole.short_user_input] when [TERM=dumb] Read full lines when asking for user input when TERM=dumb Dec 5, 2025
@kit-ty-kate
Copy link
Member

@arvidj i've squashed and renamed the commit/PR. If you're happy with it feel free to mark this PR ready to review and i'll merge it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Confusing confirmation behavior when running opam in dumb terminals

2 participants