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 from the target
and let the user modify them as needed.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3106
+ m_arguments_field = AddArgumentsField();
+ m_enviroment_field = AddEnvironmentVariableListField();
+
----------------
We should fill in any current environment variables from the target if we
aren't already doing this.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3110
+
+ m_stop_at_entry_field = AddBooleanField("Stop at entry point.", false);
+ m_working_directory_field =
----------------
Set default values for all fields in here from the target if/when possible.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3123
+ m_standard_output_field =
+ AddFileField("Standard Output", "", /*need_to_exist=*/false,
+ /*required=*/false);
----------------
Maybe label as "Redirect Standard Output" or "Standard Output File"?
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3126
+ m_standard_error_field =
+ AddFileField("Standard Error", "", /*need_to_exist=*/false,
+ /*required=*/false);
----------------
If you change above field, then modify this one as well.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3129
+ m_standard_input_field =
+ AddFileField("Standard Input", "", /*need_to_exist=*/false,
+ /*required=*/false);
----------------
If you change above field, then modify this one as well.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3140
+ m_stop_at_entry_field->FieldDelegateShow();
+ m_working_directory_field->FieldDelegateShow();
+ m_disable_aslr_field->FieldDelegateShow();
----------------
I'd pull working directory out of advanced settings.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3163
+
+ void SetArguments(ProcessLaunchInfo &launch_info) {
+ TargetSP target = m_debugger.GetSelectedTarget();
----------------
Should this be labeled "GetArguments"? The name seems like it would set the
arguments in this class from launch_info, but it is getting them from this
class and filling them into "launch_info". Ditto for all accessors that fill in
"launch_info" below.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3237-3240
+ } else {
+ action.Open(STDIN_FILENO, dev_null, true, false);
+ launch_info.AppendFileAction(action);
+ }
----------------
We don't need to do anything if this isn't specified as by default the input
will be hooked up by the debugging to something valid. If the users wants to
redirect to /dev/null, they can just specify "/dev/null" (or the right path for
this on their system.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3246-3249
+ } else {
+ action.Open(STDOUT_FILENO, dev_null, false, true);
+ launch_info.AppendFileAction(action);
+ }
----------------
Ditto above, don't do anything if this isn't specified.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3255-3258
+ } else {
+ action.Open(STDERR_FILENO, dev_null, false, true);
+ launch_info.AppendFileAction(action);
+ }
----------------
Ditto above, don't do anything if this isn't specified.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3264-3265
+
+ if (target->GetInheritTCC())
+ launch_info.GetFlags().Set(eLaunchFlagInheritTCCFromParent);
+
----------------
This is an obscure MacOS specific setting, so no need to make a field of it
like suggested below.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3267-3268
+
+ if (target->GetDetachOnError())
+ launch_info.GetFlags().Set(eLaunchFlagDetachOnError);
+
----------------
We should make a Boolean input field in the advanced settings, and the default
should be set to "target->GetDetachOnError()". Then the user can modify this if
needed.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3270-3271
+
+ if (target->GetDisableSTDIO())
+ launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
+ }
----------------
This should be moved above all of the STDIO redirection stuff above since it
will cause everything to be redirected to /dev/null or equivalent. We should
make a boolean setting for this in the advanced stuff, and if this is set to
true, then don't show the output redirection fields (m_standard_input_field,
m_standard_output_field and m_standard_error_field) in UpdateFieldsVisibility()
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107869/new/
https://reviews.llvm.org/D107869
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
