[debugger] Add P command to inspect the currently evaluted value

[alexander-yakushev]

I can't possibly count how many times I struggled to inspect the value that is right there in the debugger, right next to my cursor, teasing me with its printed representation, but impossible to inspect because I didn't let it to anything.

These times are no more. This little fix will make it possible to press P while in the debugger to inspect the value of the latest debugging step.

[alexander-yakushev]

There is an alternative approach: p inspect prompts the user for the value to be inspected. We can treat empty input as "inspect current value". One less binding, but have to tell that to user somehow (in the prompt? Inspect value (empty for this value):?)

[bbatsov]

Shouldn't name the keyword differently? This seems pretty confusing to me.

[alexander-yakushev]

I followed suit with :continue and :Continue :)

[alexander-yakushev]

:inspect-current-value is wordy and will all indentation in read-debug-command

[bbatsov]

Yeah, I just remembered this. :-) I recall I had the same misgivings about it, but space is truly at a premium.

[alexander-yakushev]

Another alternative (slightly breaking): we can switch p to inspect current value, and P to inspect... which will prompt the user for thing to be inspected. Arguably and subjectively, inspecting the current value would be a much more common action than inspecting something else (given that to inspect a local binding you can also press l and navigate to it).

[bbatsov]

I completely agree. Let's swap the two commands.

[Malabarba]

Agreed as well.

[alexander-yakushev]

Doing this turns out to be more annoying than I thought. Just adding a command with the key P puts it in front of the list because we send it as a hashmap, and somewhere inside bencode I guess it gets sorted by keys... This luckily worked fine for Continue because it just happened to end up near continue, but two inspects being far from each other looks kinda dumb.

Ideas?

[Malabarba]

Maybe it's time we start hiding some of these commands from the prompt. At some point "self-documented" starts to turn-into "confusing".
Ideally, we would have a Help key in there which would bring up a list of commands with description. But that would be beyond the scope of this PR.

Additionally, it sounds odd that the order of the commands is given by the order of keys in a hash-map. This order should probably be decided by the frontend (cider.el, in this case).

[bbatsov]

Additionally, it sounds odd that the order of the commands is given by the order of keys in a hash-map. This order should probably be decided by the frontend (cider.el, in this case).

I completely agree. I think we should tweak the UI code in general to give control to the users over what's shown be default as well.

[alexander-yakushev]

Can we perhaps start with this, and think about improving the presentation side later? I'm afraid I can't properly implement either the help window, and replacing the input-type map with a vector requires plenty of ceremonial dancing to avoid breaking changes.

[Malabarba]

Although I think such improvements are beyond the scope of this PR, I don't like that we'll have insPect at the very start of the list for an indeterminate amount of time.

Can we withhold this while someone implements at least some small fix on the Emacs side? Simply hiding Continue and insPect would already be enough for a start

[bbatsov]

Let's post some ticket on CIDER's tracker so we won't forget about it. I'm juggling a million things these days, but if there are no volunteers I can rework myself the CIDER side next week.

[bbatsov]

and replacing the input-type map with a vector requires plenty of ceremonial dancing to avoid breaking changes.

One note on this point - as CIDER is the only editor that uses the debugging functionality from cider-nrepl (to my knowledge at least) I think that breaking compatibility is not really a concern here.

[bbatsov]

FYI - we'll soon unblock this with #654

[yuhan0]

I helped to rebase the changes in this branch, see https://github.com/yuhan0/cider-nrepl/tree/inspect-current

[bbatsov]

@yuhan0 I guess you can just open a new PR, so we can finish this. I'm sure @alexander-yakushev won't mind.

[alexander-yakushev]

Sure, no problem. I'd love to see it merged.

[bbatsov]

Closing in favour of #655.