[Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1082
+      io_redirect_or_error = ScriptInterpreterIORedirect::Create(
+          options.GetEnableIO(), m_debugger, nullptr);
+
----------------
`/*result=*/nullptr`


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1208
+      io_redirect_or_error = ScriptInterpreterIORedirect::Create(
+          options.GetEnableIO(), m_debugger, nullptr);
+
----------------
`/*result=*/nullptr`


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2780
+      io_redirect_or_error = ScriptInterpreterIORedirect::Create(
+          options.GetEnableIO(), m_debugger, nullptr);
+
----------------
`/*result=*/nullptr`


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2795
                  Locker::FreeAcquiredLock |
-                     (init_session ? Locker::TearDownSession : 0));
+                     (init_session ? Locker::TearDownSession : 0),
+                 io_redirect.GetInputFile(), io_redirect.GetOutputFile(),
----------------
Not sure how I feel about using `0` as kind of a noop for an enum rather then explicitly name it as such. 


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2799
 
-  auto ExtendSysPath = [this](std::string directory) -> llvm::Error {
+  auto ExtendSysPath = [&](std::string directory) -> llvm::Error {
     if (directory.empty()) {
----------------
`&directory` so we are explicit about what we are capturing.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2896
   const bool was_imported_globally =
       (ExecuteOneLineWithReturn(
            command_stream.GetData(),
----------------
This is super confusing to read with the outer parens, it looks like the whole expression is being passed to a function.

It looks similar to an `if` statement further down.


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

https://reviews.llvm.org/D105327