[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-18 Thread Greg Clayton via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG82507f179876: [LLDB][GUI] Add Process Launch form (authored by OmarEmaraDev, committed by clayborg). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Work well now on macOS. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new/ https://reviews.llvm.org/D107869

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-18 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev updated this revision to Diff 367171. OmarEmaraDev added a comment. - Address review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new/ https://reviews.llvm.org/D107869 Files: lldb/include/lldb/Target/Target.h lldb/source/C

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So I am trying this out. I tried the target creation form and I am not able to use arrow keys when entering the path to the main executable. Are you able to do this? It seems li

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-12 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev updated this revision to Diff 365958. OmarEmaraDev added a comment. - Separate environment field into two fields. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new/ https://reviews.llvm.org/D107869 Files: lldb/include/lldb/Ta

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-12 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added a comment. Adding a boolean for inheriting the environment may not be necessary, we can just add two environment fields, one for inherited and one for user specified. The inherited one will be put in advanced settings with another boolean that show or hide the field. Both wil

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-12 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added inline comments. Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3223 + +const Environment &target_environment = target->GetEnvironment(); +m_enviroment_field->AddEnvironmentVariables(target_environment); clayborg wrote: > Does this

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D107869#2940428 , @clayborg wrote: > In D107869#2940016 , @OmarEmaraDev > wrote: > >> @jingham I wasn't arguing that we should remove those environment variables, >> on the contrary.

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3223 + +const Environment &target_environment = target->GetEnvironment(); +m_enviroment_field->AddEnvironmentVariables(target_environment); Does this currently get all tar

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D107869#2940016 , @OmarEmaraDev wrote: > @jingham I wasn't arguing that we should remove those environment variables, > on the contrary. Greg was suggesting that we populate the environment field > with the target environme

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D107869#2940016 , @OmarEmaraDev wrote: > @jingham I wasn't arguing that we should remove those environment variables, > on the contrary. Greg was suggesting that we populate the environment field > with the target environmen

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev updated this revision to Diff 365825. OmarEmaraDev added a comment. - Address review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new/ https://reviews.llvm.org/D107869 Files: lldb/source/Core/IOHandlerCursesGUI.cpp Index:

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added a comment. @jingham I wasn't arguing that we should remove those environment variables, on the contrary. Greg was suggesting that we populate the environment field with the target environment instead of implicitly adding them to the environment of the launch info. The problem

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D107869#2939922 , @OmarEmaraDev wrote: > One thing to note as well is that the target environment seem to include many > environment variables like PATH and HOME, I don't think those should be > included. If I am debugging

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added a comment. One thing to note as well is that the target environment seem to include many environment variables like PATH and HOME, I don't think those should be included. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new/

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-11 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added inline comments. Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3237-3240 +} else { + action.Open(STDIN_FILENO, dev_null, true, false); + launch_info.AppendFileAction(action); +} clayborg wrote: > We don't need to do anyt

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3105 + +m_arguments_field = AddArgumentsField(); +m_enviroment_field = AddEnvironmentVariableListField(); We should fill in any arguments automatically into this field fro

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-10 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added a comment. Herald added a subscriber: JDevlieghere. This is not fully working yet, and I am still debugging it. But I thought I would push it for early feedback regardless. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-10 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev created this revision. OmarEmaraDev added a reviewer: clayborg. Herald added a reviewer: teemperor. OmarEmaraDev requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This patch adds a process launch form. Additionally, a LazyBoolean