Filter and control order of debugging prompt overlay commands

[yuhan0]

As per the blocker on clojure-emacs/cider-nrepl#613, which is a feature I'm looking forward to :)

I rearranged the default alphabetical order of the commands to exclude Continue and group them more semantically, here's what it looks like:

---

Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

• The commits are consistent with our contribution guidelines
• You've added tests (if possible) to cover your change(s)
• All tests are passing (make test)
• All code passes the linter (make lint) which is based on elisp-lint and includes
  • byte-compilation, checkdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.
• You've updated the changelog (if adding/changing user-visible functionality)
• You've updated the user manual (if adding/changing user-visible functionality)

[bbatsov]

I think that's the wrong approach to solving the problem. As I mentioned in the original ticket I think that the debugger middleware should send back the list of supported commands, the client should decide on the key mappings internally and never send the keys back to the debugger, as it definitely doesn't need to know about those. The client and the middleware should communicate only in terms of the command names.

Is this something you'd be willing to tackle? It's pretty straightforward, I've just been extremely busy with other stuff lately.

[yuhan0]

Oh okay, I must have misunderstood the original conversation.

Just to clarify, that would mean a change to cider-nrepl to return a list of supported command names ["continue", "eval"...], and a change to cider.el to introduce a key->command mapping defvar ((?c . "continue") (?e . "eval") ...)?

In that way the communicated list can be considered an unsorted set, and it's the ordering and inclusion in the alist that would determine how the overlay is displayed?

[bbatsov]

Just to clarify, that would mean a change to cider-nrepl to return a list of supported command names ["continue", "eval"...], and a change to cider.el to introduce a key->command mapping defvar ((?c . "continue") (?e . "eval") ...)?

Yeah, exactly. Basically clients need to know only about the supported commands and they have to figure out key mappings themselves.

In that way the communicated list can be considered an unsorted set, and it's the ordering and inclusion in the alist that would determine how the overlay is displayed?

Exactly. As not all commands can be fitted on the screen probably at some point we should have some split between commands that are visible and those that are available, but hidden (e.g. they are not frequently used), but that's something for a later revision of this. The only reason why the keys are hardcoded today is that we didn't envision other clients of cider-nrepl and other commands. We were clearly wrong on both points. :D

[yuhan0]

See clojure-emacs/cider-nrepl#654,
Debugging functionality is working as before but tests are not passing for now - if everything looks alright I'll squash the previous commits and tweak the necessary tests / changelog notes

[bbatsov]

Yeah, that's exactly what I had in mind. Apart from picking better names for commands like Continue all is good. Maybe we can keep the names for now and address this later, as it might be nice to have more descriptive names, but some people might prefer to shorten those somehow (e.g. by specifying some alias) to fit more commands.

[yuhan0]

I just realised a bug with the current implementation - hidden commands like "Continue" can't be sent to the middleware, because the alist doubles as both the key mapping and the display spec.

I'll go ahead and implement the display-name mapping later this evening, would you prefer it to be a separate alist variable or combined with the current one?
ie. where the 2nd element is the command, and 3rd element is either the display name or nil for a hidden command:

'((?e "eval" "eval")
  (?C "continue-all" nil)
...)

[bbatsov]

I'd go with a single list. I guess the third element can be completely optional, but explicit nils are fine as well.

[bbatsov]

💯