Skip to content

Comments

ncurses: update to 6.4.20230708#17759

Merged
mmuetzel merged 1 commit intomsys2:masterfrom
mmuetzel:ncurses
Jul 14, 2023
Merged

ncurses: update to 6.4.20230708#17759
mmuetzel merged 1 commit intomsys2:masterfrom
mmuetzel:ncurses

Conversation

@mmuetzel
Copy link
Collaborator

That should be including a fix for the issue also addressed in #17494.

@Sonic-Amiga
Copy link

By the way, since you're updating the library, you can also fix one more problem, if you want. If $TERM variable is not empty, it discards the API driver and tries to use terminfo and control codes. But it fails to load the terminfo because default path appears to be not translated to Windows. I. e. it says "/usr/share/terminfo" instead of "C:/MSYS2/mingw64/share/terminfo". I see there's a 002-ncurses-config-win-paths.patch exactly for this purpose, but it's missing TERMINFO_DIR, or how it's called, i forgot.

Symptom of this problem: when $TERM is not empty, applications refuse to run with "Failed to open terminal".

Note, however, that this is not the only problem with this. If you fix it, you'll see the next issue. Any attempt to use control codes ($TERM is set to a proper name) will end up in garbled screen. I don't know the root cause. I remember that couple years ago, back in Windows 7 time, i had one program at work, which also used ncurses, and i ported it to Windows. And everything worked fine in both modes. But WinPTY support in MinTTY wasn't implemented yet; so mintty could only use control codes; and only native cmd.exe could use API driver. I know now things have changed on both sides; perhaps something conflicts.

I am thinking of making another patch to at least work around this problem. It's possible to detect WinPTY without relying on $TERM.

@mmuetzel
Copy link
Collaborator Author

Thank you for the feedback.

You seem to know already pretty well how to reproduce the issues that you are describing, and you have a good idea how to fix them. Feel free to open a PR with the necessary changes. 👍
It would be ideal if the fix for the second issue could (also) be sent upstream. It sounds like other projects might profit from it as well.
The relocation issue (or path translation?) on the other hand might be something that upstream might not want to care about. But a patch for MSYS2 would be appreciated.

If you don't mind, I'd like to merge this patch in its current form because it already fixes the issue tackled in #17494. The other changes could be made in follow-up PRs if you like. Feel free to ping me on those PRs.
Please, let me know if you think the other issues should be tackled before this is merged. (If no one opposes, I'll merge probably tomorrow.)

@Sonic-Amiga
Copy link

Of course, let's merge. The second problem is a bit more complex and more experimental.

@Sonic-Amiga
Copy link

Sonic-Amiga commented Jul 14, 2023

... i can post path fix for your reference, but IMHO hard error feels better than garbled screen. So perhaps should be held.

@mmuetzel
Copy link
Collaborator Author

Ah. I think I got your point now: Currently ncurses crashes. But with the recent changes it no longer crashes, but garbled text is printed because of another bug.
Did I understand correctly?

I agree that not merging is probably the better option in this case.

Closing this PR.

@mmuetzel mmuetzel closed this Jul 14, 2023
@Sonic-Amiga
Copy link

No! You're wrong.

@Sonic-Amiga
Copy link

This PR as it is would fix backspace and not do any harm.
I was suggesting more changes on top of this, which would have such an effect.

@mmuetzel
Copy link
Collaborator Author

Thanks for clarifying. From your previous comments, it wasn't clear to me which change should be held because of which reason...

Merging now.

@mmuetzel mmuetzel reopened this Jul 14, 2023
@mmuetzel mmuetzel merged commit badbc65 into msys2:master Jul 14, 2023
@Sonic-Amiga
Copy link

Sorry for thee confusion. Perhaps too much of a brain dump.

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.

2 participants