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

Already on GitHub? Sign in to your account

Different prompt for breakpoint #1310

Closed
mrak opened this Issue Feb 19, 2014 · 13 comments

Comments

Projects
None yet
6 participants
Contributor

mrak commented Feb 19, 2014

I like to be able to have prompt for a stopping at a breakpoint be different than the default. Like showing function/line number, for instance.

If there is a way to detect that you are in a "breakpoint reached" mode either through the status builtin or some other method would be great :)

Right now it's hard to see the difference between the prompt being normal and one with a breakpoint's context available

Member

zanchey commented Feb 19, 2014

Seems like a good idea!

@ridiculousfish ridiculousfish added this to the fish-future milestone Feb 20, 2014

@zanchey zanchey added the enhancement label Jul 21, 2015

I like the idea of the prompt changing to the line number when paused!

@ridiculousfish ridiculousfish modified the milestone: next-2.x, fish-future Jul 22, 2015

Owner

ridiculousfish commented Jul 22, 2015

This sounds good to me too.

Member

faho commented Mar 6, 2016

This probably won't happen for 2.3.0, retargeting.

@faho faho modified the milestone: next-2.x, 2.3.0 Mar 6, 2016

@krader1961 krader1961 modified the milestone: fish-future, fish 2.5.0 Jan 2, 2017

@krader1961 krader1961 self-assigned this Jun 19, 2017

@krader1961 krader1961 modified the milestone: fish 2.7.0, fish-future Jun 19, 2017

Member

krader1961 commented Jun 19, 2017 edited

Adding a status is-breakpoint is trivial (I already have it working). But while testing it I noticed that I can type breakpoint at an interactive prompt. Which doesn't report an error but does set $status to one. It also pushes a breakpoint block on the parser stack. That's definitely a bogosity. It shouldn't be possible to execute that command interactively. Once I merge this it will be easy enough to do something like this in fish_prompt:

if status is-breakpoint
    echo (status current-function):(status current-line)
end

Do we want to add a fish_breakpoint_prompt, analogous to fish_mode_prompt, that is always added to the output of fish_prompt? Or do we want to define a new prompt function that is used when status is-breakpoint is true rather than fish_prompt? Or, perhaps use fish_breakpoint_prompt as a substitute for fish_right_prompt if we're inside a breakpoint?

Member

krader1961 commented Jun 20, 2017

I found another bug. The breakpoint command is not a no-op in non-interactive shells.

Member

zanchey commented Jun 20, 2017

I think fish_breakpoint_prompt is a sensible idea; there's really no reason for it to be the same as fish_prompt and that confused the heck out of me the first time I used a breakpoint.

Member

krader1961 commented Jun 20, 2017

I'm leaning towards using fish_breakpoint_prompt as a replacement for fish_right_prompt (rather than fish_prompt) when in breakpoint context. That's because a) I suspect few people are using a right prompt and b) those that are using one are likely displaying information like the current time that isn't interesting when debugging. Having to duplicate large parts of the left prompt in a debug prompt is likely to be more painful. I would also consider naming this new function fish_debug_prompt but I like the obvious tie to the breakpoint command by naming it fish_breakpoint_prompt.

Member

krader1961 commented Jun 20, 2017

On the other hand using a custom right prompt and leaving the left prompt the user's standard prompt may not make the fact we're in the debugger sufficiently clear. Nonetheless, I'm going to implement it as a right prompt replacement. We can easily change it to be a left prompt replacement if user feedback suggests that is necessary.

@krader1961 krader1961 added a commit to krader1961/fish-shell that referenced this issue Jun 20, 2017

@krader1961 krader1961 implement `status is-breakpoint`
This implements `status is-breakpoint` that returns true if the current
shell prompt is displayed in the context of a `breakpoint` command.

This also fixes several bugs. Most notably making `breakpoint` a no-op if
the shell isn't interactive. Also, typing `breakpoint` at an interactive
prompt should be an error rather than creating a new nested debugging
context.

Partial fix for #1310
776de94

@krader1961 krader1961 added a commit to krader1961/fish-shell that referenced this issue Jun 20, 2017

@krader1961 krader1961 implement `status is-breakpoint`
This implements `status is-breakpoint` that returns true if the current
shell prompt is displayed in the context of a `breakpoint` command.

This also fixes several bugs. Most notably making `breakpoint` a no-op if
the shell isn't interactive. Also, typing `breakpoint` at an interactive
prompt should be an error rather than creating a new nested debugging
context.

Partial fix for #1310
a703be1
Member

krader1961 commented Jun 20, 2017

I've tested using the breakpoint prompt as an alternative right prompt. This is probably not viable because the length of the debug prompt is likely to be long enough that the entire right prompt gets suppressed. Even if we only display the function name and line number. It's practically certain to be suppressed if we include the filename -- even if just limited to a fraction of the pathname. I tested this on a terminal 90 chars in width with a moderately long left prompt. So the debug prompt will have to be an alternative for the left prompt. If people want to include components of their left prompt in the debug prompt they will have to define their own fish_breakpoint_prompt function.

@krader1961 krader1961 added a commit to krader1961/fish-shell that referenced this issue Jun 20, 2017

@krader1961 krader1961 implement the `fish_breakpoint_prompt` function
This is another step to resolving issue #1310. It makes
`fish_breakpoint_prompt` a replacement for `fish_prompt` if it is defined
and we're presenting a prompt in the context of a `breakpoint` command.
89c4e37
Member

krader1961 commented Jun 20, 2017 edited

Note that we need a way to get the context in which a breakpoint command was executed. The current status current-* functions do not provide this. So I'm going to add -B / --breakpoint flags to specify that is the context of interest when executing status. At this time the proposed debug prompt out for review does not provide the correct context. Thus that change is a proof of concept intended to verify the basic approach. That is, using an alternative left prompt and using the fish_color_status var when displaying breakpoint context.

@krader1961 krader1961 added a commit to krader1961/fish-shell that referenced this issue Jun 20, 2017

@krader1961 krader1961 implement the `fish_breakpoint_prompt` function
This is another step to resolving issue #1310. It makes
`fish_breakpoint_prompt` a replacement for `fish_prompt` if it is defined
and we're presenting a prompt in the context of a `breakpoint` command.
8ee27e8

@krader1961 krader1961 added a commit that referenced this issue Jun 21, 2017

@krader1961 krader1961 implement `status is-breakpoint`
This implements `status is-breakpoint` that returns true if the current
shell prompt is displayed in the context of a `breakpoint` command.

This also fixes several bugs. Most notably making `breakpoint` a no-op if
the shell isn't interactive. Also, typing `breakpoint` at an interactive
prompt should be an error rather than creating a new nested debugging
context.

Partial fix for #1310
bd299e9

@krader1961 krader1961 added a commit that referenced this issue Jun 21, 2017

@krader1961 krader1961 implement the `fish_breakpoint_prompt` function
This is another step to resolving issue #1310. It makes
`fish_breakpoint_prompt` a replacement for `fish_prompt` if it is defined
and we're presenting a prompt in the context of a `breakpoint` command.
df01547

@krader1961 krader1961 added a commit that referenced this issue Jun 21, 2017

@krader1961 krader1961 provide shorter `status` subcommands
As part of addressing #1310 I decided it makes more sense to replace
`current-function` with just `function`, etc., because I'm going to add
flags to let the user specify which stack level they are interested in.
With the default being zero or the "current" level.
e26f7aa
Member

krader1961 commented Jun 21, 2017

For those keeping track I have merged changes which will result in a distinct fish prompt when asking for user input in the context of a breakpoint. The information in that prompt will be incorrect because we currently do not have a way to get the correct information from within a prompt function. While imperfect this is a huge improvement over the previous situation which presented the user with a prompt that was extremely misleading.

@krader1961 krader1961 added a commit that referenced this issue Jun 24, 2017

@krader1961 krader1961 implement `status function` when in a breakpoint
Another step to fixing #1310. This changes means that `status -L0
function` reports the correct function when inside a breakpoint.
30368d5
Member

krader1961 commented Jul 1, 2017

Closing this issue since I've fixed it. The user now sees a very distinctive prompt when a breakpoint command is executed. That prompt includes the correct function name and line number of the breakpoint.

There is more work to be done to make status useful in other contexts such as a hypothetical trap DEBUG context which would allow creating more flexible alternative to the set -x of other shells. But that work isn't relevant to this issue.

@krader1961 krader1961 closed this Jul 1, 2017

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