clayborg added a comment.
Looks very nice.
A few things to consider here, and I am open to everyone's opinion here, did
you consider implementing each field as a Window? It seems that a lot of the
window code is duplicated, DrawBox for one. I know we would need to make
modifications to the Window class if this, but just something to think about.
It might be more trouble that it is worth.
Another question I have is if we want a box around each field and do we want
the user to be able to select the exact location of each item? Right now we can
make a nice UI with almost window like fields, but I worry that if we have a
lot of fields and they don't fit in the window, how complex it the scrolling
code going to be? Another way would be to make a list of fields. And you add
them in the order you want them to show up in the list. There could be no boxes
around each field in this case and the window could look more like a list in
this case. I will try and show with some ASCII art what I was thinking:
/--------------------------------------------------------------------\
| Path: /tmp/a.out |
| Number: 1234 |
| Boolean: true |
| Choices: Choice 1 |
|--------------------------------------------------------------------|
| [ SUBMIT ] |
\--------------------------------------------------------------------/
This might make it easier to implement scrolling if there are too many entries
to fit on the screen.
Future things we can add would include:
- a field grouping field that would not take any input, but it would allow
fields to be grouped into sections
- subclasses of the text field that allow for things like paths which might
auto complete and allow only files or directories to be selected based on
constructor args
- an array of a specific kind of fields, like for program arguments that would
take a FieldDelegate and allow for a list to be created
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:432-440
+ void PutCStringTruncatedWidth(int right_pad, const char *s, int width,
+ int len = -1) {
+ int bytes_left = width - GetCursorX();
if (bytes_left > right_pad) {
bytes_left -= right_pad;
::waddnstr(m_window, s, len < 0 ? bytes_left : std::min(bytes_left,
len));
}
----------------
move this below function below PutCStringTruncated so that PutCStringTruncated
doesn't appear to be edited.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:713
+ PutChar('[');
+ PutCString(title);
+ PutChar(']');
----------------
Do we need to use PutCStringTruncated here just in case?
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:935-937
+ : m_label(label), m_width(width), m_origin(origin), m_content(content),
+ m_cursor_position(0), m_first_visibile_char(0) {
+ assert(m_width > 2);
----------------
"m_content(content):" will crash if content is NULL.
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:962
+
+ int GetContentLength() { return (int)m_content.length(); }
+
----------------
Does this need to be an integer? Can we return "size_t"?
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1060
+ // Returns the text content of the field.
+ std::string GetText() { return m_content; }
+
----------------
Return const reference to avoid copy. If use wants to copy it, they can assign
to a local
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1108
+ window.AttributeOn(A_REVERSE);
+ window.PutChar(m_content ? 'X' : ' ');
+ if (is_active)
----------------
We could use ACS_DIAMOND or ACS_BULLET? Check it out and see how it looks?
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1120
+ switch (key) {
+ case ' ':
+ ToggleContent();
----------------
enter or return key as well? or is that saved for submit in the main window?
================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1442
+ // The index of the selected field. This can be equal to the number of
fields,
+ // in which case, it denotes that the button is selected.
+ int m_selected_field_index;
----------------
The submit button?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104395/new/
https://reviews.llvm.org/D104395
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
