[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/include/lldb/Target/Platform.h:643
+          *command_output, // Pass nullptr if you don't want the command output
+      const Timeout<std::micro> &timeout, bool run_in_shell = true);
 
----------------
mib wrote:
> labath wrote:
> > The run_in_shell argument makes the inferface weird (why do I need to specify a shell, if I don't want to use it). And still suffers from the virtual default argument problem described above. Let's not propagate this Host weirdness into the platform class.
> I started by removing this argument, then I noticed that some code calls `Host::RunShellCommand` with the `run_in_shell` argument set to false (i.e. `PlatformDarwin::GetXcodeSelectPath`). This means that `Host::RunShellCommand` will run the command without prepending the "sh -c" part.
> 
> That's why I kept it, and renamed it to `run_in_shell` instead of `run_in_default_shell`.
Right, but that code only calls the Host version. I'm only saying we should delete it (or, not add it, to be precise) to the Platform version. (I'd like to remove it from the Host version too, but that's a different story.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667