[PATCH] D95230: Save and restore previous terminal after setting the terminal for checking if terminal supports colors.
Pavel Labath via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 4 05:37:13 PST 2021
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
This change seems obviously correct. We'll still need to be careful about using this functionality on the lldb side though. This fix will only help if there is proper synchronization between using this function and libedit initialization. If we still run into issues, we'll need to either add more synchronization, or do something different altogether.
Regarding testing, I suppose it should be possible to write a unit test which checks that this function really preserves the terminal value, but it's not clear to me how useful would that be....
================
Comment at: llvm/lib/Support/Unix/Process.inc:357-358
// Now extract the structure allocated by setupterm and free its memory
// through a really silly dance.
+ struct term *termp = set_curterm(previous_term);
----------------
Now that its purpose is also to restore the terminal value, this code is no longer that "silly".
I'd update this to something like:
```
// Restore the current terminal to the original value. Be sure to also free the structure allocated by setupterm.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95230/new/
https://reviews.llvm.org/D95230
More information about the llvm-commits
mailing list