On some Linux distributions, after setting up the EditLine object, the eventual call to set_curterm, which happens when calculating terminal properties, breaks the libedit configuration, causing characters that have functions bound to them not to show up. As a workaround, we calculate these terminal properties eagerly, before initializing the EditLine object.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This issue was found on the Swift REPL. However, I tested @teemperor's C REPL patch (https://reviews.llvm.org/D87281) and the same problem shows up.
Can you be more specific about what is the nature and cause of this breakage?
Could it maybe be the fact that the set_curterm(nullptr) call (inside terminalHasColors(fd)) leaves terminfo with no "current" terminal, (whereas before the call it contained the "current" terminal, as setup by libedit? If so then maybe the right fix is to change terminalHasColors to restore the current terminal to whatever it was at the start of the function.
@labath, you were absolutely correct! It was simply a matter of saving and restoring the terminal struct on the terminalHasColors function in the Process.inc file (I really should've tried that before). I'm currently recompiling and will re-run the tests locally, and will push the changes after that. I do worry this could potentially impact macOS though (I don't know if these low-level terminal functions work differently between differently OSes), so how do we ensure this doesn't break anything there?
Also, since the patch will be completely different, should I open a new patch or push to this one and just change the title/description accordingly?
We should pick reviewers which have some knowledge of this -- the original author of that code + whoever reviewed that patch is a good starting point. If this breaks anyone's use case after it gets reviewed&committed, that person will surely let us know (and then we can figure out what to do). However, I don't think this will be a particularly risky change. The more interesting question is whether there is any reasonable way of testing this....
Also, since the patch will be completely different, should I open a new patch or push to this one and just change the title/description accordingly?
A new patch would probably be better in this case. However, it might be a good idea to add a reference to this discussion for context.
clang-format not found in user's PATH; not linting file.