Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a widget to list all breakpoints #318

Closed
spyder-bot opened this issue Feb 16, 2015 · 21 comments
Closed

Create a widget to list all breakpoints #318

spyder-bot opened this issue Feb 16, 2015 · 21 comments

Comments

@spyder-bot
Copy link
Collaborator

From mproel...@googlemail.com on 2010-08-27T01:12:29Z

Hi,

this is rather an enhancement:

it would be nice to have all breakpoints of all open files in a new "breakpoint widget" for instance in form of a table

file | activate | condition | comment

filename+line with link | check box | a condition | don't know wheter
| | | a comment is useful?

and please with keyboard support :)
a shortcut to jump to the widget the arrow keys to navigate between colums/rows and the space to jump to breakpoint/activate the breakpoint

Original issue: http://code.google.com/p/spyderlib/issues/detail?id=318

@spyder-bot
Copy link
Collaborator Author

From pierre.raybaut on 2010-08-29T01:21:05Z

This could be useful indeed, but not my top priority (you may list all breakpoints directly in pdb with the 'b' command)

Status: Accepted
Labels: -Type-Defect -Priority-Medium Type-Enhancement Priority-Low

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2011-10-07T13:40:09Z

Summary: Create a widget to list all breakpoints
Labels: Cat-Debugger

@spyder-bot
Copy link
Collaborator Author

From pierre.raybaut on 2011-10-29T04:33:25Z

Status: ContributorNeeded

@spyder-bot
Copy link
Collaborator Author

From steve.f....@gmail.com on 2011-11-08T11:39:52Z

Can this maybe be addressed with the other debugger issues? This would be extremely useful.

@spyder-bot
Copy link
Collaborator Author

From pierre.raybaut on 2012-03-18T14:10:10Z

Status: HelpNeeded
Labels: -Type-Enhancement Type-Enh

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-05-12T18:22:12Z

Ok, I've implemented a very simple breakpoint plugin. It doesn't implement all of the features request in the original report but it's a start. I would consider it useable but very much a work in progress. The code can be pulled from my clone at bookmark jedludlow-issue-318 here: https://code.google.com/r/jedludlow-spyderlib-default/source/browse/?name=jedludlow-issue-318 The clone is up to date with all other changes from the default repo. I welcome any testing or code review.

First, I implemented it as a third-party plugin for now because I wanted to make as little impact on the core source as possible. It would be straightforward to migrate it to a standard plugin later if desired.

Second, I made a deliberate decision to use the breakpoints dictionary in the config file as the source for the breakpoints data so that the plugin might be helpful in diagnosing things like issue #634 and issue #849 . This decision made the implementation a little cheesy for now. Fundamentally, breakpoints are marked in the code editor as block data tied to source code lines but they are then aggregated across all code editors and stored to the config file in a dictionary. The main problem is that the breakpoint line numbers change with edits to the code, and those changes aren't reflected in the breakpoints dictionary until a "breakpoints_changed()" signal is emitted. So I implemented a recurring timer that emits "breakpoints_changed()" every 10 sec. It's not the way to go for the long term, but a different approach would require more fundamental changes to the way breakpoints are handled inside spyder, and I didn't want to undertake those changes without more discussion.

Status: Started
Owner: jed.lud...@gmail.com
Cc: ccordoba12 pierre.raybaut

@spyder-bot
Copy link
Collaborator Author

From steve.f....@gmail.com on 2012-05-12T18:26:06Z

Sounds nice. I will try to start testing this week.

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-05-12T18:54:03Z

Great work Jed! It's really nice to see some improvements on this area (that, as I said, I think it's our weakest one). I don't have time right now to test it but I hope to do it tomorrow.

Labels: -Priority-Low Priority-High

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-06-29T07:05:11Z

Jed, I'm ready to review and merge your changes. A couple of comments:

  1. Instead of a timer, would it be possible to emit the signal when the number of lines of a file change? Maybe that could be a better and simpler approach, I don't know.
  2. How hard would it be to add click support to the elements of your table, so the user could can jump to the file and line where the breakpoint is? I'm afraid that will be the first thing our users will ask for after they see the new plugin.
  3. I would like to see and activate column, as suggested by mproeller in his first comment, but that could be implemented after we merge the plugin. How hard do you thing it could be to develop?

In any case, great work. It'll be a great addition for 2.2!

Labels: MS-v2.2

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-06-29T07:47:19Z

Answers to the questions from comment 9:

  1. As I mentioned above, I'm not particularly happy with the timer, but I'm also not convinced that I know exactly what event to use to trigger a rescan of the file for break point locations. Maybe it's number of lines in the file.
  2. There is already double-click support. If you double click any line in the table it takes you to the file and break point location.
  3. This would require adding an enable attribute to break points which does not yet exist. I agree that it's a good idea, but I elected to implement something simpler to start with that didn't require fundamental changes to the break point structure.

I'm not sure it's ready for broader release. I honestly think it's worth reviewing the break point design in general. See my notes here: https://code.google.com/p/spyderlib/wiki/BreakpointDesign Right now the break point list is implemented as a 3rd party plugin, but it probably belong in the spyder plugins category instead if there is broad demand for it.

Honestly, it's going to be very difficult for me to make much contribution for a while so if someone wants to contribute enhancements go for it.

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-06-29T09:07:14Z

My answers:

  1. Yep, I also think number of lines in the file would be the best signal to emit.
  2. I thought single-click was enough. It's really cool :-)
  3. Maybe we could undertake the improvement task you propose in 2.3. I don't have time right now to dig into this and if you won't be available to discuss/implement changes, then it would be even harder to make progress. I'll be in favor to merge this as is, solve any issues we observe during the 2.2 cycle and with that input try to develop a better solution for 2.3. What do you think?

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-06-29T09:09:04Z

By the way, are there other debugging improvements you consider ready for 2.2? I mean, besides this one and issue #554

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-06-30T09:22:04Z

This issue was updated by revision 57c46147d921 .

  • This implements a widget to list all breakpoints from all open files in the
    Editor.
  • It also adds a global shortcut (Ctrl+B) to show the plugin.
  • There are still some things to be developed, like the possibility to activate/
    deactivate breakpoints from the plugin and to select breakpoints using the
    arrow keys and Enter.

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-06-30T09:23:06Z

Jed, I went ahead and merged your plugin so we can continue improving it in the main tree.

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-07-01T15:23:06Z

This issue was updated by revision 9694e58b0f73 .

  • Introduce a new breakpoints attribute to the CodeEditor class.
  • Now we save breakpoints only when this attribute changes.

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-07-01T15:23:08Z

This issue was updated by revision d5df2c73ca05 .

  • Got rid of the timer used before and try to update if there are changes in the
    the number of lines in the file.
  • Also compare the list of saved breakpoints against the current ones before
    trying to save them to disk.

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-07-02T20:13:31Z

Revision d5df2c73ca05 implements an excellent change that removes the timer, but it introduced a couple of problems as described in the code review here: https://code.google.com/p/spyderlib/source/detail?r=d5df2c73ca05b3c23e7071e1438e470478a2b589 The attached patch moves the break point checking back to the editor widget.

Attachment: breakpoints.patch

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-07-03T06:24:44Z

This issue was updated by revision 181a9d4581f8 .

  • This partially reverts revision d5df2c73ca05 , which broke saving and updating
    breakpoints from the config dictionary.

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-07-03T06:27:44Z

Thanks Jed for the patch, I tested and it works great. I just added a bit more info to your commit message.

One more thing: What do you think about the update_breakpoints method? Should we remove it now that it's only emitting a signal?

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-07-03T07:12:13Z

I think it's fine to leave it as is for today. I can see how we might eventually do more than just emit a signal later on.

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-12-19T08:09:06Z

Status: Verified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant