python: Let pdb tracking not kill buffers

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

python: Let pdb tracking not kill buffers

Andrii Kolomoiets
Hello,

Let's start from example:
1. echo "import pdb; pdb.set_trace()" > test.py
2. emacs -Q
3. M-x run-python
4. M-x python-shell-send-file <RET> test.py <RET>

Now there are two windows: one with pdb session and another one with
source code.
Now in pdb prompt: pass<RET>

The source code buffer is killed because pdb tracking comint output
filter doesn't found file name in the output and decides that tracking
session is over.

This behavior interferes with debug session. Moving frame up/down the
stack trace open new files but evaluating some code kill them when they
are still needed.

Another reason to not kill buffers is that I want the file opened during
debug session stay there when the debug session is over because I want
to change it.

My point is: it's hard to tell that debug session is over judging from
the output but it can be determined by inspecting the input.

Attached patch brings the following changes:

- New variable `python-pdbtrack-kill-buffers' that make buffers killing
  optional;

- Comint input filter which decides that the debug session is over;

- Process sentinel to finish tracking when python process is killed.

Please see attached patch. I certainly sure docstrings and naming are
not so good but they can be fine tuned later if the main idea will be
accepted.

Thanks.

pdb-track.patch (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Eli Zaretskii
> From: Andrii Kolomoiets <[hidden email]>
> Date: Fri, 4 Oct 2019 23:32:09 +0300
>
> 1. echo "import pdb; pdb.set_trace()" > test.py
> 2. emacs -Q
> 3. M-x run-python
> 4. M-x python-shell-send-file <RET> test.py <RET>
>
> Now there are two windows: one with pdb session and another one with
> source code.
> Now in pdb prompt: pass<RET>
>
> The source code buffer is killed because pdb tracking comint output
> filter doesn't found file name in the output and decides that tracking
> session is over.
>
> This behavior interferes with debug session. Moving frame up/down the
> stack trace open new files but evaluating some code kill them when they
> are still needed.

Thanks.

Aren't users supposed to use pdb via "M-x pdb" instead?  (I don't use
this, and don't debug Python programs, so maybe my question makes no
sense.)

> Attached patch brings the following changes:
>
> - New variable `python-pdbtrack-kill-buffers' that make buffers killing
>   optional;
>
> - Comint input filter which decides that the debug session is over;
>
> - Process sentinel to finish tracking when python process is killed.
>
> Please see attached patch. I certainly sure docstrings and naming are
> not so good but they can be fine tuned later if the main idea will be
> accepted.

Besides the question I asked above, your patch is too large to be
accepted without assigning copyright to the FSF.  Would you like to
start the legal paperwork rolling, so that any contributions from you
could be accepted without limitations?

> +(defcustom python-pdbtrack-continue-command '("c" "cont" "continue")
> +  "Pdb continue command."
> +  :type 'list
> +  :group 'python)

Each new defcustom should have a :version tag.  Also, if they belong
to the group of the current file, our convention is not to use :group,
as that's redundant.

In addition, please try making the doc strings explain how is the
variable used and what is its effect, including (if applicable) the
effect of special values, like nil etc.

> +(defcustom python-pdbtrack-kill-buffers t
> +  "Kill buffers when tracking is finished.
> +Only buffers opened during tracking will be killed."

The first sentence should be "If non-nil, kill buffers when tracking
is finished."  (And that is somewhat unclear, because it isn't clear
what it means "when tracking is finished".)

Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Xiangqian Liu
Dear

I am a full time python developer and seems in my case mostly I am using the way as below:

insert a 

import pdb; pdb.set_trace();

in my code bass and trigger the break point from other place (mostly browser) and wait for the break point been hit. 

Thanks. 

Eli Zaretskii <[hidden email]>于2019年10月5日 周六14:41写道:
> From: Andrii Kolomoiets <[hidden email]>
> Date: Fri, 4 Oct 2019 23:32:09 +0300
>
> 1. echo "import pdb; pdb.set_trace()" > test.py
> 2. emacs -Q
> 3. M-x run-python
> 4. M-x python-shell-send-file <RET> test.py <RET>
>
> Now there are two windows: one with pdb session and another one with
> source code.
> Now in pdb prompt: pass<RET>
>
> The source code buffer is killed because pdb tracking comint output
> filter doesn't found file name in the output and decides that tracking
> session is over.
>
> This behavior interferes with debug session. Moving frame up/down the
> stack trace open new files but evaluating some code kill them when they
> are still needed.

Thanks.

Aren't users supposed to use pdb via "M-x pdb" instead?  (I don't use
this, and don't debug Python programs, so maybe my question makes no
sense.)

> Attached patch brings the following changes:
>
> - New variable `python-pdbtrack-kill-buffers' that make buffers killing
>   optional;
>
> - Comint input filter which decides that the debug session is over;
>
> - Process sentinel to finish tracking when python process is killed.
>
> Please see attached patch. I certainly sure docstrings and naming are
> not so good but they can be fine tuned later if the main idea will be
> accepted.

Besides the question I asked above, your patch is too large to be
accepted without assigning copyright to the FSF.  Would you like to
start the legal paperwork rolling, so that any contributions from you
could be accepted without limitations?

> +(defcustom python-pdbtrack-continue-command '("c" "cont" "continue")
> +  "Pdb continue command."
> +  :type 'list
> +  :group 'python)

Each new defcustom should have a :version tag.  Also, if they belong
to the group of the current file, our convention is not to use :group,
as that's redundant.

In addition, please try making the doc strings explain how is the
variable used and what is its effect, including (if applicable) the
effect of special values, like nil etc.

> +(defcustom python-pdbtrack-kill-buffers t
> +  "Kill buffers when tracking is finished.
> +Only buffers opened during tracking will be killed."

The first sentence should be "If non-nil, kill buffers when tracking
is finished."  (And that is somewhat unclear, because it isn't clear
what it means "when tracking is finished".)

--
Best Regards
Lawrence
Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Andrii Kolomoiets
In reply to this post by Eli Zaretskii
> On Oct 5, 2019, at 09:40, Eli Zaretskii <[hidden email]> wrote:
>
> Aren't users supposed to use pdb via "M-x pdb" instead?  (I don't use
> this, and don't debug Python programs, so maybe my question makes no
> sense.)

M-x pdb requires launch program in gud-mode while pdbtrack comint output
filter can be used in any buffer and mode, like interactive python
shell.

>> Please see attached patch. I certainly sure docstrings and naming are
>> not so good but they can be fine tuned later if the main idea will be
>> accepted.
>
> Besides the question I asked above, your patch is too large to be
> accepted without assigning copyright to the FSF.  Would you like to
> start the legal paperwork rolling, so that any contributions from you
> could be accepted without limitations?

Yes, please.

>> +(defcustom python-pdbtrack-continue-command '("c" "cont" "continue")
>> +  "Pdb continue command."
>> +  :type 'list
>> +  :group 'python)
>
> Each new defcustom should have a :version tag.  Also, if they belong
> to the group of the current file, our convention is not to use :group,
> as that's redundant.

Would it be better to create a separate subgroup for pdbtracking related
variables?

>> +(defcustom python-pdbtrack-kill-buffers t
>> +  "Kill buffers when tracking is finished.
>> +Only buffers opened during tracking will be killed."
>
> The first sentence should be "If non-nil, kill buffers when tracking
> is finished."  (And that is somewhat unclear, because it isn't clear
> what it means "when tracking is finished".)

Please see updated patch.


pdb-track.patch (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Eli Zaretskii
In reply to this post by Xiangqian Liu
> From: Lawrence Liu <[hidden email]>
> Date: Mon, 7 Oct 2019 12:33:17 +0800
> Cc: Andrii Kolomoiets <[hidden email]>, [hidden email]
>
> I am a full time python developer and seems in my case mostly I am using the way as below:
>
> insert a
>
> import pdb; pdb.set_trace();
>
> in my code bass and trigger the break point from other place (mostly browser) and wait for the break point
> been hit.

Any reason you don't use "M-x pdb" instead?

Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Xiangqian Liu
I think the main reason is because sometimes,

the entires of our program is not a python file, it might be launch of a docker image, a gunicorn application, even a background task etc. or it might be wrapped by some 3rd launcher.  

And sometimes it will coming through many steps before hit the break point. 


Eli Zaretskii <[hidden email]>于2019年10月8日 周二00:19写道:
> From: Lawrence Liu <[hidden email]>
> Date: Mon, 7 Oct 2019 12:33:17 +0800
> Cc: Andrii Kolomoiets <[hidden email]>, [hidden email]
>
> I am a full time python developer and seems in my case mostly I am using the way as below:
>
> insert a
>
> import pdb; pdb.set_trace();
>
> in my code bass and trigger the break point from other place (mostly browser) and wait for the break point
> been hit.

Any reason you don't use "M-x pdb" instead?
--
Best Regards
Lawrence
Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Andrii Kolomoiets
In reply to this post by Andrii Kolomoiets
On 7 Oct 2019, at 15:28, Andrii Kolomoiets <[hidden email]> wrote:
>
>> On Oct 5, 2019, at 09:40, Eli Zaretskii <[hidden email]> wrote:
>>
>> Besides the question I asked above, your patch is too large to be
>> accepted without assigning copyright to the FSF.  Would you like to
>> start the legal paperwork rolling, so that any contributions from you
>> could be accepted without limitations?
>
> Yes, please.

Copyright paperwork is done.


Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Eli Zaretskii
> From: Andrii Kolomoiets <[hidden email]>
> Date: Wed, 30 Oct 2019 21:14:38 +0200
> Cc: [hidden email]
>
> Copyright paperwork is done.

Thanks.

Can you please add:

  . Commit log message formatted according to instructions in
    CONTRIBUTE
  . Suitable short entry for NEWS about the new options

Also, I don't see any rationale for changing
python-pdbtrack-stacktrace-info-regexp; can you tell why you needed
that?

Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Andrii Kolomoiets
On 1 Nov 2019, at 11:35, Eli Zaretskii <[hidden email]> wrote:
> Can you please add:
>
>  . Commit log message formatted according to instructions in
>    CONTRIBUTE
>  . Suitable short entry for NEWS about the new options

Please see attached updated patch.

> Also, I don't see any rationale for changing
> python-pdbtrack-stacktrace-info-regexp; can you tell why you needed
> that?

Current python-pdbtrack-stacktrace-info-regexp match only lines with real
filenames, like:
  > /Users/mad/test.py(3)<module>()->None

Pdb command "next" can bring us to upper stack frame, like:
  > <stdin>(1)<module>()->None

At this point pdbtracking session can be considered over.  And new
python-pdbtrack-stacktrace-info-regexp can match such lines with "<stdin>"
or "<string>" as filenames.


0001-python.el-Pdbtracking-improvements.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Eli Zaretskii
> From: Andrii Kolomoiets <[hidden email]>
> Date: Sat, 2 Nov 2019 18:37:26 +0200
> Cc: [hidden email]
>
> On 1 Nov 2019, at 11:35, Eli Zaretskii <[hidden email]> wrote:
> > Can you please add:
> >
> >  . Commit log message formatted according to instructions in
> >    CONTRIBUTE
> >  . Suitable short entry for NEWS about the new options
>
> Please see attached updated patch.

Thanks.

> Current python-pdbtrack-stacktrace-info-regexp match only lines with real
> filenames, like:
>   > /Users/mad/test.py(3)<module>()->None
>
> Pdb command "next" can bring us to upper stack frame, like:
>   > <stdin>(1)<module>()->None

OK, but the change in that variable should be called out in the log
entry, so please add that, together with the explanation above.  Maybe
also add a comment before the variable to explain what it needs to
match.

> +*** New user option 'python-pdbtrack-kill-buffers'.
> +If nil, buffers opened during pdbtracking session is not killed when
                                                     ^^
"are", plural.

Thanks.

Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Andrii Kolomoiets
On 2 Nov 2019, at 19:29, Eli Zaretskii <[hidden email]> wrote:

>
>> Current python-pdbtrack-stacktrace-info-regexp match only lines with real
>> filenames, like:
>>> /Users/mad/test.py(3)<module>()->None
>>
>> Pdb command "next" can bring us to upper stack frame, like:
>>> <stdin>(1)<module>()->None
>
> OK, but the change in that variable should be called out in the log
> entry, so please add that, together with the explanation above.  Maybe
> also add a comment before the variable to explain what it needs to
> match.
Explanation is added to python-pdbtrack-stacktrace-info-regexp docstring.

>> +*** New user option 'python-pdbtrack-kill-buffers'.
>> +If nil, buffers opened during pdbtracking session is not killed when
>                                                     ^^
> "are", plural.

Fixed.

Thanks.


0001-python.el-Pdbtracking-improvements.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: python: Let pdb tracking not kill buffers

Eli Zaretskii
> From: Andrii Kolomoiets <[hidden email]>
> Date: Sat, 2 Nov 2019 20:51:13 +0200
> Cc: [hidden email]
>
> On 2 Nov 2019, at 19:29, Eli Zaretskii <[hidden email]> wrote:
> >
> >> Current python-pdbtrack-stacktrace-info-regexp match only lines with real
> >> filenames, like:
> >>> /Users/mad/test.py(3)<module>()->None
> >>
> >> Pdb command "next" can bring us to upper stack frame, like:
> >>> <stdin>(1)<module>()->None
> >
> > OK, but the change in that variable should be called out in the log
> > entry, so please add that, together with the explanation above.  Maybe
> > also add a comment before the variable to explain what it needs to
> > match.
>
> Explanation is added to python-pdbtrack-stacktrace-info-regexp docstring.
>
> >> +*** New user option 'python-pdbtrack-kill-buffers'.
> >> +If nil, buffers opened during pdbtracking session is not killed when
> >                                                     ^^
> > "are", plural.
>
> Fixed.

Thanks, pushed.