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

Allow the %run magic with '-b' to specify a file. #2782

Merged
merged 2 commits into from Jan 13, 2013

Conversation

ellbur
Copy link
Contributor

@ellbur ellbur commented Jan 13, 2013

Hello,

Here is a minor tweak to the %run command that some users might find helpful:

The syntax would be:

%run -d -b file2.py:30 file1.py

to break at line 30 of file2.py when running file1.py.

The old syntax

%run -d -b 20 file1.py

still works the way it used to.

Suggested by user gozzilli on StackOverflow
(http://stackoverflow.com/questions/14305203/ipython-set-a-breakpoint-in-imported-file/).

@@ -578,7 +582,7 @@ def run(self, parameter_s='', runner=None,
ns = {'execfile': py3compat.execfile, 'prog_ns': prog_ns}
try:
#save filename so it can be used by methods on the deb object
deb._exec_filename = filename
deb._exec_filename = bp_file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this one be adjusted? I'll just test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I can't produce a situation where _exec_filename is actually used. Can you? If not, I think we shouldn't change it - it seems more likely that it is supposed to be the name of the top-level file that's being run, rather than the file the breakpoint is in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you're right. I didn't actually know what it was there for, so
I have no real reason to change it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The debugger uses it when looking for lines if the code was compiled with the filename <string>. But I don't know how to create a case where that happens.

Do you want to push another commit to the same branch reversing the change on this line?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, now I can get it, if I exec a string of code inside the module. I don't think there's a way of being certain about it in that case, but I still think we should leave it as filename for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this should be left as filename.

@bfroehle
Copy link
Contributor

Other than the one change that @takluyver and I agree on, this is ready to go.

I looked through the pdb source hoping that there would a function we could reuse instead of parse_breakpoint, but came up empty. (The code is in pdb.Pdb.do_break and is significantly core complicated since it allows for breaking at functions...).

@bfroehle
Copy link
Contributor

An alternative approach, which might have been simpler, would have been something more like:

bp_cmd = opts.get('b', [1])[0]
if ':' not in bp_cmd:
    bp_cmd = filename + ':' + bp_cmd
# ...
deb.do_break(bp_cmd)

But I'm not sure if deb.do_break would have run through all of the same checkline code that we do. As a side effect, this might have allowed breaking at function names (but I never implemented it to test it, so there might be other unexpected issues which would prevent that from working anyway).

For now I think the proposed patch here is plenty fine.

The syntax would be:
    %run -d -b file2.py:30 file1.py

to break at line 30 of file2.py when running file1.py.

The old syntax
    %run -d -b 20 file1.py
still works the way it used to.

Suggested by user gozzilli on StackOverflow
(http://stackoverflow.com/questions/14305203/ipython-set-a-breakpoint-in-imported-file/).
@ellbur
Copy link
Contributor Author

ellbur commented Jan 13, 2013

OK, I changed bp_file back to filename. Thanks for looking at this :)

@takluyver
Copy link
Member

OK, great. We'd normally want to add a test as well, especially as %run is one of the most delicate bits of the codebase. But I don't see an easy way to write an automated test for this.

@bfroehle has already OKed this as well, so I'm going to merge it now.

takluyver added a commit that referenced this pull request Jan 13, 2013
Allow the %run magic with '-b' to specify a file.
@takluyver takluyver merged commit cdc7e66 into ipython:master Jan 13, 2013
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Allow the %run magic with '-b' to specify a file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants