@clayborg Did you take a look at this?

An example typical workflow for navigating to a file and adding a breakpoint.

@clayborg I see a number of issues with using forms directly:

@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?

An example searcher window for source files:

• Address review

@clayborg Perhaps you missed this, it is essential the same as D107182 but without the unsupported function. It would be good to have this committed.

• Separate environment field into two fields.

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 will be filled with the appropriate default values. What do you think?

• Address review.

@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 with that is that there is a large number of environments that gets added (15 in my case) as shown in the following screenshot. What I was saying in my comment above is that I don't think we should show those to the user, we should transparently add them instead and let the user add additional variables as needed. We can then look into adding other settings to exclude them if necessary.

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.

This is not fully working yet, and I am still debugging it. But I thought I would push it for early feedback regardless.

@clayborg This is unrelated to D107761, so it should be safe to commit.

• Fix possible crash in SetText

This is a reimplementation of D107182, which was committed and then reverted because the is_pad function is not universally supported. The updated patch adds an explicit type member to identify the backing object of the surface.

@jingham I see. Thanks!

• Address review

@clayborg With you suggestions it currently looks like this. The concern I have is that lines are now 36 characters longer, and some important information are now truncated. The breakpoint and breakpoint locations IDs seems to be just the index of the objects in the tree, are they not? In this case, they seem redundant to me. Similarly, the address takes a lot of space and is included in the next level of details, so it might not be as important to include in this level either.

@jasonmolenda Okay, I will just reimplement the patch without is_pad somehow.

is_pad was added in 2009 in ncurses 5.7 - patch 20090906. So it is sufficiently old to be used, is it not?

@jasonmolenda is_pad is defined by ncurses. According to the man page, it is an extension that is not supported on Version 7, BSD or System V implementations., but that shouldn't be a problem as far as I know. Can you maybe check the ncurses header and see if it is there?

@clayborg What do you think? Should we add another tree level with more details or do something else?

I faced a bit of difficulty in this patch, so it is not exactly complete. One issue is that breakpoint descriptions can be very long, I tried to implemented multiline items to workaround this, but that didn't work out and was a waste of time. For now, I just tried to keep descriptions as small as possible. One thing I want to try now is to simply add another tree level and add all necessary information there, similar to verbose breakpoint description.
This is also missing the breakpoint operators, like enabling and disabling breakpoints. Since the window doesn't delegate key handing to the items, this will have to be implemented first.

I generalized the implementation as suggested and added a ready to use list version of the field.

• Generalize the implementation to any Mapping field

Example list of environment fields:

• Address review comments

I still can't get remote debugging to work unfortunately, or maybe I don't understand it really. The way I understand it is as follows:

• Rebase on main.
• Add basic remote debugging support.

Note that all the GUI tests spawn a terminal of dimensions (100, 500), not the default (24,80). And I am guessing this includes the control characters as well. So even 2000 may not be enough even.

@clayborg Did you check D106564? This is the last patch needed to do the rebase.

• Merge main and fix conflicts.

• Rebase on main and fix conflicts.

• Remove default arguments

Are you going to land the other smaller diffs first and then update this one after hey have landed?

Thanks @DavidSpickett! I will look into this and let you know how it goes.

I am currently breaking this patch into smaller independent viable patches as suggested.

This doesn't support remote files yet, I am still having trouble testing those. Also, there is also an unrelated clang-format change, not sure if I should revert it or keep it.

• Manually continue if needed.

Two actions with an option:

• Return raw pointers instead of unique ones.
• Add Show Advance Settings option.
• Allow detaching and killing at the same time.
• Allow detaching while keeping process stopped.
• Handle review comments.

Updated UI:

• Add kill/detach form.
• Fix forms with no fields.

Currently working on the detach/kill form.

• Use unique pointers for field delegates.
• Fix typos in Select methods.
• Set the process name to the main executable name by default.
• Turn plugin name into a choice field.
• Update debugger target if a new one was created.
• Remove superfluous continue operation.

I initially created two forms for attach by name and attach by PID, because the options were divided between them. Today I tried to reimplement that such that it is a single form with a choices field at the top that determines if it is by name or PID. The fields are then shown or hidden based on that enum. Additionally, other fields can control the visibility of other fields, for instance, the "Include existing processes" field. I think it works fine and the UX looks good. Here is an example, what do you think?

Full example:

• Add contextual scrolling support.

• Scrolling was temporarily removed from the patch. It was causing issues with fields that change in size. I will reimplement it as contextual scrolling directly.
• Action buttons weren't moved to the window border as discussed. The window border is already highlighted when the form is active, which makes highlighting and navigating fields not user friendly. Action buttons are now scrollable though, which solves the issue of space.

• Rewrite internal field navigation.
• Rewrite form window drawing. Form delegate no longer have drawing routines.
• Add global error messages.
• Add action bar. Form delegate can now define as many arbitrary actions as needed.
• Make action button scrollable.
• Add support for composite fields.
• Add backward tab navigation.

Not available in this patch yet:

• Global error messages.
• Contextual scrolling.
• Action bar.
• Auto completion.

• Add Surface type.
• Add Pad and SubPad types.
• Implement dynamic scrolling.
• Implement per-field validation and error messages.
• Implement File field.
• Implement Directory field.
• Implement List field.
• Refactor field drawing.

Since we are still working on this diff. I will add the other form functionality I have been working on here as well if you don't mind.

• Remove Field type and use FieldDelegate directly

@clayborg I tried implementing scrolling mechanisms as suggested. My first trial essentially defined a "visible area" rectangle which gets updated with every time the selection changes, then when it comes to drawing, each field checks if it is completely contained in the visible area and draws itself with an offset that we get from from the visible area origin. This worked, but fields that spans multiple columns can completely disappear leaving mostly no fields on the window, so it worked in most cases, but not all. My second trial was about drawing to an ncurses pad that is large enough to fit all contents, then the window is refreshed from that pad. This used manual ncurses calls because we don't support pads at the moment, so I scratched that for now. I think support for pads would be good for those kind of applications in the future. I plan to work on a proposal that would include support for pads and lightweight subwindows, I will detail that later.

• Add form pages.
• Handle review.

• Always scroll left on removing a character

• Remove PutCStringTruncatedWidth, use a character limit instead.
• Handle review.

did you consider implementing each field as a Window?

An example form:

• Handle review

• Merge branch 'main' into lldb-gui-expand-threads-tree
• Implement default selection

• Follow LLVM coding guidelines