mgorny marked an inline comment as done.
mgorny added inline comments.
================
Comment at: lldb/source/Host/common/Terminal.cpp:39
+
llvm::Expected<Terminal::Data> Terminal::GetData() {
if (!FileDescriptorIsValid())
----------------
thakis wrote:
> nit: should GetData() even exist if `!LLDB_ENABLE_TERMIOS`? Looks like it's
> only called `#if LLDB_ENABLE_TERMIOS` now. Maybe the whole method definition
> should be inside an ifdef.
>
> If it should keep existing, either move the !FileDescriptorIsValid() check
> into the #if inside it (imho preferred), or move all the callers of it
> outside that #if. The current mix is a bit self-inconsistent.
I think the goal was to avoid checking `LLDB_ENABLE_TERMIOS` in the header
file, though I'm not sure if it was an explicit goal or one I've implicitly
assumed.
I've move the check inside for the time being. We can always change it later.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112632/new/
https://reviews.llvm.org/D112632
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
