[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

@clayborg I was planning on getting to field completion later as part of a global "context window" feature. There are reasons why I need this as a full separate window for now. Imminently, I am creating an operator that changes the file in the sources window so that breakpoints can be inserted manually. So once the user press Ctrl+F, a search window will appear where the user can choose a source file to navigate to. This will require a custom searcher, and not the common searcher implemented above, which I will add in a following patch. What do you think?



================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3592
+        m_selected_match(0), m_first_visible_match(0) {
+    ;
+  }
----------------
clayborg wrote:
> Should we be doing just like any other dialog that we have created and be constructing these items on the fly?
> 
> ```
> m_text_field = AddTextField("Search", "", true);
> ```
> It seems the matches could use a modified ChoicesField for the matches? Are you currently drawing the choices (matches) manually?
`AddTextField` is part of form delegates, I don't think implementing this as a form is a good idea as they are functionally distinct.

I am drawing choices manually because the duplicated code is not really a lot and I like to be able to control the style of the drawing. But I guess we can reimplemented that using a choices field.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3731
+  SearcherDelegateSP m_delegate_sp;
+  TextFieldDelegate m_text_field;
+  // The index of the currently selected match.
----------------
clayborg wrote:
> Is this a new text field itself, or designed to be a reference to an existing text field? 
A new instance of a text field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545