[Lldb-commits] [PATCH] D106458: [LLDB][GUI] Add required property to text fields

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Nice smaller changes!



================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1079
+  TextFieldDelegate(const char *label, const char *content,
+                    bool required = false)
+      : m_label(label), m_required(required), m_cursor_position(0),
----------------
I would remove the default parameter to make sure everyone sets this correctly. It also will help avoid compilation issue if we ever add more parameters in the future as this can sometimes cause implicit conversion bugs.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1279
 public:
-  IntegerFieldDelegate(const char *label, int content)
-      : TextFieldDelegate(label, std::to_string(content).c_str()) {}
+  IntegerFieldDelegate(const char *label, int content, bool required = false)
+      : TextFieldDelegate(label, std::to_string(content).c_str(), required) {}
----------------
Remove default argument value for "required" as mentioned above.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1292
   FileFieldDelegate(const char *label, const char *content,
-                    bool need_to_exist = true)
-      : TextFieldDelegate(label, content), m_need_to_exist(need_to_exist) {}
+                    bool need_to_exist = true, bool required = false)
+      : TextFieldDelegate(label, content, required),
----------------
Remove default argument value for "required" as mentioned above.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1322
   DirectoryFieldDelegate(const char *label, const char *content,
-                         bool need_to_exist = true)
-      : TextFieldDelegate(label, content), m_need_to_exist(need_to_exist) {}
+                         bool need_to_exist = true, bool required = false)
+      : TextFieldDelegate(label, content, required),
----------------
Remove default argument value for "required" as mentioned above.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1876
+  TextFieldDelegate *AddTextField(const char *label, const char *content,
+                                  bool required = false) {
+    TextFieldDelegate *delegate =
----------------
Remove default argument value for "required" as mentioned above.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1884-1885
   FileFieldDelegate *AddFileField(const char *label, const char *content,
-                                  bool need_to_exist = true) {
+                                  bool need_to_exist = true,
+                                  bool required = false) {
     FileFieldDelegate *delegate =
----------------
Remove default argument value for "need_to_exist" and "required" as mentioned above. Have everyone set them each time.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1894-1895
                                             const char *content,
-                                            bool need_to_exist = true) {
+                                            bool need_to_exist = true,
+                                            bool required = false) {
     DirectoryFieldDelegate *delegate =
----------------
Remove default argument value for "need_to_exist" and "required" as mentioned above. Have everyone set them each time.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1903
+  IntegerFieldDelegate *AddIntegerField(const char *label, int content,
+                                        bool required = false) {
+    IntegerFieldDelegate *delegate =
----------------
Remove default argument value for "required" as mentioned above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106458