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

Unbind C-c ? and C-c C-h (maybe) #273

Closed
mattiase opened this issue Sep 2, 2021 · 22 comments
Closed

Unbind C-c ? and C-c C-h (maybe) #273

mattiase opened this issue Sep 2, 2021 · 22 comments

Comments

@mattiase
Copy link
Contributor

mattiase commented Sep 2, 2021

Emacs expects ?, C-h and <f1> to be unbound in prefix maps because these keys are used to display a list of commands beginning with that prefix. For instance, M-s C-h normally shows all keys starting with M-s and so on.

Unfortunately Tuareg binds C-c ? and C-c C-h, leaving only <f1> for looking at the C-c space. This hampers discoverability a bit. The prefix maps (C-c in particular) are often crowded and shared by multiple packages, which is why this feature can be quite useful both for users and developers.

I understand that these bindings have been with Tuareg for a long time and some people may have them in their muscle memory; if you think that is more important, do close this issue as WONTFIX.

@erikmd
Copy link
Contributor

erikmd commented Sep 2, 2021

Hi all,

@mattiase

Emacs expects ?, C-h and <f1> to be unbound in prefix maps because these keys are used to display a list of commands beginning with that prefix. For instance, M-s C-h normally shows all keys starting with M-s and so on.

Unfortunately Tuareg binds C-c ? and C-c C-h, leaving only for looking at the C-c space. This hampers discoverability a bit. The prefix maps (C-c in particular) are often crowded and shared by multiple packages, which is why this feature can be quite useful both for users and developers.

I believe that's a very good point: Emacs' native C-h suffix feature is indeed very handy to increase the discoverability…

BTW, I've seen in the Emacs' manual page "D.2 Key Binding Conventions" that they include this recommendation:

Don’t bind C-h following any prefix character (including C-c). If you don’t bind C-h, it is automatically available as a help character for listing the subcommands of the prefix character.

But, instead of "unbinding" caml-help, maybe one could just "rebind" the command caml-help to C-c M-h, for example?

Given with this mere rebinding, if some users often relied on C-c C-h, I guess they wouldn't be that surprised: as they would instead get a list of the keybindings… including the new one (C-c M-h :)

What do you think?

(FWIW, I'm not suggesting to use the simpler keybinding C-c h because of this other Emacs' manual recommendation:

Don’t define C-c letter as a key in Lisp programs. Sequences consisting of C-c and a letter (either upper or lower case) are reserved for users; they are the only sequences reserved for users, so do not block them. )

@erikmd
Copy link
Contributor

erikmd commented Sep 2, 2021

However, I believe it's unneeded to change anything regarding C-c ? (if I'm not mistaken)

@erikmd
Copy link
Contributor

erikmd commented Sep 2, 2021

Actually, it seems both caml-mode and tuareg-mode are concerned:

@mattiase
Copy link
Contributor Author

mattiase commented Sep 3, 2021

C-c ? is defined in both tuareg-mode and tuareg-interactive-mode (the REPL) and doesn't quite work in either, so perhaps these bindings haven't seen much use lately. It's tuareg-interactive-error-range-regexp that hasn't kept up with changes to the error message format, which also affects error highlighting in the REPL. This could be causing #248.

It also affects tuareg-eval-region, which apart from having its binding usurped by merlin-error-check also suffers from its position calculation being off, again affecting error parsing.

I'll see what can be done about these problems.

mattiase added a commit to mattiase/tuareg that referenced this issue Sep 3, 2021
Set `tuareg-interactive-last-phrase-pos-in-source` to the actual start
of the region sent to the REPL. Previously, the incorrect position
caused error locations to be off, causing
`tuareg-interactive-next-error-source` to fail (see ocaml#273, ocaml#231).

Rewrite to follow the spirit of the original code: extend the region
to contain all phrases it intersects.
@Chris00
Copy link
Member

Chris00 commented Sep 4, 2021

I'm fine with binding caml-help to something else — C-cC-h was indeed inherited from caml-mode. I'm not too keen on C-cM-h as I find it cumbersome to type. Maybe C-cC-y — although it is less mnemonic or C-cC-?? What are other modes using?

mattiase added a commit to mattiase/tuareg that referenced this issue Sep 4, 2021
Set `tuareg-interactive-last-phrase-pos-in-source` to the actual start
of the region sent to the REPL. Previously, the incorrect position
caused error locations to be off, causing
`tuareg-interactive-next-error-source` to fail (see ocaml#273, ocaml#231).

Rewrite to follow the spirit of the original code: extend the region
to contain all phrases it intersects.
mattiase added a commit to mattiase/tuareg that referenced this issue Sep 4, 2021
Set `tuareg-interactive-last-phrase-pos-in-source` to the actual start
of the region sent to the REPL. Previously, the incorrect position
caused error locations to be off, causing
`tuareg-interactive-next-error-source` to fail (see ocaml#273, ocaml#231).

Rewrite to follow the spirit of the original code: extend the region
to contain all phrases it intersects.
@monnier
Copy link
Contributor

monnier commented Sep 4, 2021 via email

@mattiase
Copy link
Contributor Author

mattiase commented Sep 4, 2021

All the good bindings seem to be taken. Maybe C-c C-o, for dOc?

And yes, I too wish Emacs had a standard way for showing documentation in programming modes.

@erikmd
Copy link
Contributor

erikmd commented Sep 4, 2021

Hi all,

All the good bindings seem to be taken. Maybe C-c C-o, for dOc?

maybe… or maybe not 😅
− In the tuareg+merlin config we distribute to our students, we're precisely binding C-c C-o to merlin-iedit-occurrences (as occurrences starts with o :)
because the very handy merlin-iedit mode does not hard-code a key-binding itself.

So, counterproposal: would you be fine for binding C-c ? to caml-help ?

I see that in the fairly standard AUCTeX mode, we have the binding:

Signature:
(TeX-documentation-texdoc &optional ARG)

Key Bindings:
TeX-mode-map C-c ?

The advantage is that the meaning of ? is obvious, and it doesn't hinder the recommendation mentioned above: Don’t define C-c letter (either upper or lower case) as a key in Lisp programs., as ? is not a letter :)

Incidentally, it appears tuareg currently uses this binding, fortunately with another existing, synonymous binding C-c `:

Signature:
(tuareg-interactive-next-error-source)

Key Bindings:
tuareg-mode-map C-c ?
tuareg-mode-map C-c `

So even if this should be changed as well, I guess that'd be OK because C-c ` is more "intuitive" than C-c ? for "next error": according to the standard global binding C-x `

WDYT?

@erikmd
Copy link
Contributor

erikmd commented Sep 4, 2021

Sorry @mattiase, I've just realized you had already suggested the same, in your previous comment:

C-c ? is defined in both tuareg-mode and tuareg-interactive-mode (the REPL)
[…]

but this doesn't change my opinion that rebinding C-c ? would really make sense 🙂

Kind regards

@mattiase
Copy link
Contributor Author

mattiase commented Sep 5, 2021

Good bindings are indeed hard to come by. I note that Merlin also has the unbound command merlin-document.

I've looked at other packages and predictably, there is no consensus about keybindings or anything else. The closest appears to be C-c C-d which seems fairly popular for documentation-related things, and it rolls off the fingers reasonably well, but of course Merlin uses it for merlin-destruct.

Ideally Emacs would have a standard (short! one key!) binding for a mode-specific describe-thing-at-point, with an extendible framework for actually displaying the information (in a window, in the echo area, in a popup overlay, etc) and with standard faces. Tuareg & friends would then use it to show the type, signature, parameter names and doc comment (markup-processed, not raw like merlin-document does it).

At least the above is what you would expect from any remotely modern programmer's editor.

It may also be useful to have a standard for other documentation-related commands, or a prefix for a collection of such commands. Several packages use C-c C-d as a prefix.

@erikmd
Copy link
Contributor

erikmd commented Sep 9, 2021

Hi, indeed it seems all good solutions we could find for rebinding C-c C-h will need to rebind another keybinding as well (either C-c ? or C-c C-d in particular).

But after the comments above by @mattiase

C-c ? is defined in both tuareg-mode and tuareg-interactive-mode (the REPL) and doesn't quite work in either, so perhaps these bindings haven't seen much use lately […]

and mine:

I see that in the fairly standard AUCTeX mode, we have the binding: […] (TeX-documentation-texdoc &optional ARG) → C-c ?

and tuareg currently uses this binding, fortunately with another existing, synonymous binding C-c `:
(tuareg-interactive-next-error-source) → C-c ` and C-c ?

it seems that this latter choice would be at the same time natural, and causing the least harm (because C-c ` is already available, and intuitive).

So, if you don't object for replacing C-c C-h with C-c ?, I can open a PR if need be :)

mattiase added a commit to mattiase/tuareg that referenced this issue Sep 10, 2021
Set `tuareg-interactive-last-phrase-pos-in-source` to the actual start
of the region sent to the REPL. Previously, the incorrect position
caused error locations to be off, causing
`tuareg-interactive-next-error-source` to fail (see ocaml#273, ocaml#231).

Rewrite to follow the spirit of the original code: extend the region
to contain all phrases it intersects.
@mattiase
Copy link
Contributor Author

Well, failing the introduction of a standard Emacs key binding (don't hold your breath) I'd favour C-c C-d and asking the Merlin maintainers very politely if they can think of an alternative key for merlin-destruct. Maybe C-c C-e, for "explode" in the engineering sense of an exploded-view drawing that shows how an assembly is composed of parts?

However there are multiple candidates. Here's a question: if there were a single standard Emacs key for "display help for what is under the cursor without prompting the user for further information", what would you bind it to?

  • caml-help
  • merlin-document
  • merlin-type-enclosing
  • something else?

@Chris00
Copy link
Member

Chris00 commented Sep 10, 2021

Here's a question: if there were a single standard Emacs key for "display help for what is under the cursor without prompting the user for further information", what would you bind it to?

For me, it would be merlin-document if available, caml-help as a fallback.

@monnier
Copy link
Contributor

monnier commented Sep 10, 2021 via email

@mattiase
Copy link
Contributor Author

BTW, we could treat the doc as a kind of "definition", and thus add the doc as a possible choice in M-..

Yes, and I had a brief chat with Dmitry about it. It doesn't really looks like it fits with the framework or the concept. And the prospect of a universal single-keystroke context-sensitive documentation key is tempting, and has proven very useful in other programming environments.

It would also water down M-.: where it previously would have jumped to the definition directly, we would now have to choose between that and the docs. Better to keep the operations separate.

@monnier
Copy link
Contributor

monnier commented Sep 10, 2021 via email

@Chris00
Copy link
Member

Chris00 commented Sep 11, 2021

All the good bindings seem to be taken. Maybe C-c C-o, for dOc?

I was thinking that C-c C-o would be good for merlin-occurrences.

@mattiase
Copy link
Contributor Author

merlin-occurrences seems to work essentially like xref-find-references, bound to M-?. Maybe they should be unified?

@Chris00
Copy link
Member

Chris00 commented Sep 11, 2021

I agree (I discovered M-? after posting this 😀). This is part of the issue I am writing for Merlin.

@catern
Copy link

catern commented May 17, 2023

How about C-h .? That's what eglot uses.

@monnier
Copy link
Contributor

monnier commented May 17, 2023 via email

erikmd added a commit to erikmd/tuareg that referenced this issue Aug 31, 2023
erikmd added a commit to erikmd/tuareg that referenced this issue Aug 31, 2023
erikmd added a commit to erikmd/tuareg that referenced this issue Aug 31, 2023
@erikmd
Copy link
Contributor

erikmd commented Oct 3, 2023

@monnier I think this issue (addressed by PR #309) can be closed.

@monnier monnier closed this as completed Oct 3, 2023
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

No branches or pull requests

5 participants