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

mib added a comment. Should be fixed with 0e86f390457a2b4dd1f2d1770db912963a36f240 . @max-kudr I'll keep an eye on the Windows bot to make sure it worked :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

max-kudr added inline comments. Comment at: lldb/test/API/commands/platform/basic/myshell.c:11 + +#ifdef WIN32 + char *cmd_opt = "/C"; mib wrote: > @max-kudr It seems I'm missing the leading `_` (correct me if I'm wrong but > IIUC the macro should be

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

mib added inline comments. Comment at: lldb/test/API/commands/platform/basic/myshell.c:11 + +#ifdef WIN32 + char *cmd_opt = "/C"; @max-kudr It seems I'm missing the leading `_` (correct me if I'm wrong but IIUC the macro should be `_WIN32`), explaining the

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

mib added a comment. @max-kudr Looking, thanks! 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

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

max-kudr added a comment. Hi @mib, Windows buildbot is broken after this commit. Please fix or revert. Failed Tests (2): lldb-api :: commands/platform/basic/TestPlatformCommand.py lldb-api :: commands/platform/basic/TestPlatformPython.py

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaddb5148f58d: [lldb/Target] Add custom interpreter option to `platform shell` (authored by mib). Changed prior to commit:

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

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thanks, this looks good to me now. Comment at: lldb/test/API/commands/platform/basic/TestPlatformPython.py:16 mydir = TestBase.compute_mydir(__file__) +

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

mib updated this revision to Diff 288636. mib added a comment. Replaced all the `interpreter` occurrences with `shell` in the code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86667/new/ https://reviews.llvm.org/D86667 Files:

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

mib updated this revision to Diff 288597. mib edited the summary of this revision. mib added a comment. Removed `bool run_in_shell` argument from `Platform` classes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86667/new/

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

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 , bool run_in_shell = true); mib wrote: > labath wrote: > > The run_in_shell

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

mib updated this revision to Diff 288593. mib edited the summary of this revision. mib added a comment. - Changed CommandObject option from `-i|--interpreter` to `-s|--shell`. - Updated test program build settings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

mib marked 3 inline comments as done. mib 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 , bool run_in_shell = true); labath

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

jasonmolenda added a comment. In D86667#2243878 , @labath wrote: > In D86667#2243781 , @mib wrote: > >> In D86667#2242566 , @jingham wrote: >> >>> Do people really call

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

labath added a comment. In D86667#2243781 , @mib wrote: > In D86667#2242566 , @jingham wrote: > >> Do people really call command-line shells interpreters? I would have >> thought --shell would be a better name.

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

mib updated this revision to Diff 288531. mib marked an inline comment as done. mib edited the summary of this revision. mib added a comment. Address most comments: - Passed down the shell interpreter path down to `Host::RunShellCommand` to be "assembled" with the rest of the command. - Wrote a

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

mib marked 9 inline comments as done. mib added a comment. In 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

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

jingham added a comment. 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. Repository: rG LLVM

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

labath added a comment. I am wondering what should the platform classes which do not implement this feature do (although most of them could implemented I don't think you need to implement all of them -- PlatformGdbRemote in particular can be a bit tricky). It seems to me like it would be

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

kastiglione added inline comments. Comment at: lldb/source/API/SBPlatform.cpp:59 + +if (command_interpreter && command_interpreter[0]) { + full_command += command_interpreter; JDevlieghere wrote: > Given that this pattern repeats a few times in this

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

JDevlieghere added inline comments. Comment at: lldb/source/API/SBPlatform.cpp:59 + +if (command_interpreter && command_interpreter[0]) { + full_command += command_interpreter; Given that this pattern repeats a few times in this struct, maybe a small

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

mib updated this revision to Diff 288147. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86667/new/ https://reviews.llvm.org/D86667 Files: lldb/bindings/interface/SBPlatform.i lldb/include/lldb/API/SBPlatform.h

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

mib created this revision. mib added reviewers: labath, JDevlieghere, jingham. mib added a project: LLDB. Herald added subscribers: lldb-commits, dang. mib requested review of this revision. This patch adds the ability to use a custom interpreter with the `platform shell` command. If the user