[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Is anyone passing save_process_group = true now? If not, I'd really like to delete it, as the implementation is quite scary.



================
Comment at: lldb/include/lldb/Host/Terminal.h:51
 /// descriptor and later restore that state as it originally was.
 class TerminalState {
 public:
----------------
mgorny wrote:
> labath wrote:
> > I guess it would be good to delete the copy operations as well.
> This is now implicit via PImpl; or do you prefer explicit?
implicit is fine


================
Comment at: lldb/include/lldb/Host/Terminal.h:130
+  int m_tflags = -1;            ///< Cached tflags information.
+  std::unique_ptr<Impl> m_data; ///< Platform-specific implementation.
   lldb::pid_t m_process_group = -1; ///< Cached process group information.
----------------
Maybe call the class `Data` as well?


================
Comment at: lldb/source/Host/common/Terminal.cpp:92
+    : m_tty(term) {
+  Save(term, save_process_group);
 }
----------------
Could we make `Save` private now? Or even inline it into the constructor ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110721/new/

https://reviews.llvm.org/D110721