mib marked 9 inline comments as done.
mib added a comment.
In D86667#2242566 <https://reviews.llvm.org/D86667#2242566>, @jingham wrote:
> Do people really call command-line shells interpreters? I would have thought
> --shell would be a better name. lldb already has its own command interpreter
> which is orthogonal to the shells, so it seems confusing to use the same term
> for both.
I think `platform shell --shell` sounds/looks repetitive so I opted for
`-i|--interpreter` instead, as in Command-line **Interpreter**.
================
Comment at: lldb/source/API/SBPlatform.cpp:59
+
+ if (command_interpreter && command_interpreter[0]) {
+ full_command += command_interpreter;
----------------
labath wrote:
> kastiglione wrote:
> > JDevlieghere wrote:
> > > Given that this pattern repeats a few times in this struct, maybe a small
> > > static helper function would be nice:
> > >
> > > ```static bool is_not_empty(cont char* c) { return c && c[0]; }```
> > Can these be `StringRef`, and then check with `empty()`?
> Rather than duplicating this logic here (and getting it wrong for windows,
> for nested quotes, etc.) it would be better to just pass the custom shell to
> the `RunShellCommand` method. Then it use the existing quoting logic, and all
> that needs to change is that it now picks up the shell from the argument (if
> passed) instead of the baked-in default.
I think since this method will eventually interact with the Python API, they
need to be C strings.
================
Comment at: lldb/test/API/commands/platform/basic/TestPlatformCommand.py:109
+ self.assertTrue(err.Success())
+ self.assertIn("/bin/sh", sh_cmd.GetOutput())
+
----------------
kastiglione wrote:
>
Since it's a command output, it has a newline character at the end of it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86667/new/
https://reviews.llvm.org/D86667
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
