Opened 11 years ago

Closed 11 years ago

#9777 closed enhancement (fixed)

Improve breakpoint list editing

Reported by: bonefish Owned by: anevilyak
Priority: normal Milestone: Unscheduled
Component: Applications/Debugger Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

Removing or disabling several breakpoints from/in the breakpoints list is rather tedious, since one has to click and remove/disable them one by one. The list should allow multi-selection. Moreover the Delete key should remove selected breakpoints.

Change History (6)

comment:1 by anevilyak, 11 years ago

This one potentially gets a bit more messy due to the dichotomy of displaying both breakpoints and watchpoints. Those are currently independent subclasses since they have nearly nothing in common apart from being [un]installable, and consequently having the BreakpointsView show them at all is somewhat of a hack.

Perhaps it might make more sense to separate watchpoints to their own tab/view? That would clean up the Breakpoints[List]View code, and simplify handling the multiple selection cases for both, as well as allowing the watchpoint-specific view to show more detailed information about watchpoints than it currently can.

comment:2 by bonefish, 11 years ago

A few thoughts:

  • I would break out the watchpoints into a separate tab only, if it makes sense from UI point of view. Code design shouldn't guide that decision. A reason for keeping breakpoints and watchpoints in one list is that one normally doesn't have a lot of either. I don't know what additional detailed information you're thinking about displaying for watchpoints, but I guess the same can be done in a joined table. The column names might have to be generalized a bit.
  • The table model classes lack editing capabilities. With those it would be a lot easier to implement table editing operations (at least removing and moving rows) in a general way.
  • ... particularly with a helper proxy model that joins a list of other models. This way separate models could be implemented for breakpoints and watchpoints.

in reply to:  2 ; comment:3 by anevilyak, 11 years ago

Replying to bonefish:

I don't know what additional detailed information you're thinking about displaying for watchpoints, but I guess the same can be done in a joined table. The column names might have to be generalized a bit.

In the case of a breakpoint, the information of interest is the source location the breakpoint is at. For a watchpoint however, the pertinent information that I had in mind was the address, size and type (read/write) of watchpoint. Other than enabled/disabled state, there really isn't any overlap between the two, so to me it would seem a bit messy UI-wise to try and have both in the same table with redundant/unused columns just to be able to accomodate that.

  • The table model classes lack editing capabilities. With those it would be a lot easier to implement table editing operations (at least removing and moving rows) in a general way.
  • ... particularly with a helper proxy model that joins a list of other models. This way separate models could be implemented for breakpoints and watchpoints.

That would indeed be nice/helpful in general, but given the above, I really don't think it makes handling this particular case any cleaner. It's also not just the models in general that are the issue on the code side, but also e.g. selection listener hooks and such.

in reply to:  3 ; comment:4 by bonefish, 11 years ago

Replying to anevilyak:

Replying to bonefish:

I don't know what additional detailed information you're thinking about displaying for watchpoints, but I guess the same can be done in a joined table. The column names might have to be generalized a bit.

In the case of a breakpoint, the information of interest is the source location the breakpoint is at. For a watchpoint however, the pertinent information that I had in mind was the address, size and type (read/write) of watchpoint. Other than enabled/disabled state, there really isn't any overlap between the two, so to me it would seem a bit messy UI-wise to try and have both in the same table with redundant/unused columns just to be able to accomodate that.

I think, at least partially this can be remedied by generalizing the column names/meaning. The source file and line column (could be be joined BTW) would indeed not be applicable for watchpoints. The "Function" column, however, could be renamed to "Location" and could contain the relevant info for watchpoints (e.g. "read at 0xf00ba1 (4 bytes)").

Note, that I certainly don't think this is the perfect or even best solution, but I'd really like to avoid a "Watchpoints" tab. We don't have many tabs yet, but there are plenty of team-wide entities/concepts that may get their own tab eventually (global variables, areas, signals, locking primitives,...), that having separate ones for break- and watchpoints seems excessive. Showing two list views in the breakpoints page (at the same time) is a no-go due to the very limited space, so there would have to be a means to switch between them -- a combo box or radio-style toolbar buttons.

in reply to:  4 comment:5 by anevilyak, 11 years ago

Status: newin-progress

Replying to bonefish:

I think, at least partially this can be remedied by generalizing the column names/meaning. The source file and line column (could be joined BTW) would indeed not be applicable for watchpoints. The "Function" column, however, could be renamed to "Location" and could contain the relevant info for watchpoints (e.g. "read at 0xf00ba1 (4 bytes)").

That sounds potentially workable, will see what I can come up with.

comment:6 by anevilyak, 11 years ago

Resolution: fixed
Status: in-progressclosed

Implemented (hopefully) as discussed in hrev45700, let me know what you think. A behavioral note for when multiple breakpoints are selected: If at least one member of the selection is disabled, the Enable/Disable toggle will stay in Enable mode. Only if the entire selection is currently active will it toggle to allow Disable.

Note: See TracTickets for help on using tickets.