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

Emacs bindings #1386

Closed
Chris00 opened this issue Sep 11, 2021 · 9 comments · Fixed by #1668 or ocaml/opam-repository#24499
Closed

Emacs bindings #1386

Chris00 opened this issue Sep 11, 2021 · 9 comments · Fixed by #1668 or ocaml/opam-repository#24499

Comments

@Chris00
Copy link
Member

Chris00 commented Sep 11, 2021

After discussing with @monnier, @mattiase, and @erikmd about Tuareg documentation bindings, we would like to expand this discussion with the Merlin developers.

  • There is unfortunately no standard Emacs' documentation keybinding but we are thinking to use C-c C-d (we cannot use C-c C-h as C-h is reserved for discoverability). This however conflicts with the key for m︭erlin-d︭estruct. May we suggest that C-c M-d be used for this (which is even more mnemonic 😀).
  • Merlin shadows C-c C-r (bound in Tuareg and Caml-mode to ...-eval-region). What do you think of binding merlin-error-check to C-c M-r?
  • Given that M-. (resp. M-,) are standard in Emacs (and already supported by Merlin), one can remove C-c C-l (resp. C-c &) — I'm thinking to do the same in Tuareg after making caml-types-show-ident available through xref.
  • How about binding merlin-occurrences to M-? (through xref)?
  • Contrarily to what is asserted on the wiki, C-c t is no longer bound to merlin-type-expr (a good thing as it is reserved to the user). May we suggest to bind it to C-c C-t with a prefix?
@mattiase
Copy link
Contributor

And as an alternative binding for merlin-destruct, C-c C-e seems to be free -- E for Explode?

@erikmd
Copy link
Contributor

erikmd commented Sep 11, 2021

And as an alternative binding for merlin-destruct, C-c C-e seems to be free -- E for Explode?

I don't think so: it is already bound (and AFAICT, C-c C-e is a very useful keybinding in Tuareg :)

@mattiase
Copy link
Contributor

My mistake, sorry. I always use C-M-x because it's faster to type!

@Chris00
Copy link
Member Author

Chris00 commented Sep 11, 2021

It is also bound to C-x C-e (which is not harder to type than C-c C-e, contrarily to C-M-x for me), so we could free that binding...

@erikmd
Copy link
Contributor

erikmd commented Sep 11, 2021

Hi @Chris00,

It is also bound to C-x C-e (which is not harder to type than C-c C-e, contrarily to C-M-x for me), so we could free that binding...

Indeed! so to sum up the suggestions regarding merlin-destruct, it might be bound, for example:

  • to C-c M-d
  • or to C-c C-e, keeping C-M-x and C-x C-e as the binding for tuareg-eval-phrase (albeit it can be noted that C-x C-e shadows the global command eval-last-sexp which may be handy to evaluate elisp fragments in any mode (including sexp's in OCaml comments for example 😅))
  • or to something else, maybe C-c | (?)

@bbatsov
Copy link

bbatsov commented Jun 21, 2022

I think that C-c M-d is a good option for merlin-destruct and C-c C-d would better for something documentation related. I would not mess with classic keybindings from evaluating stuff.

Btw, I've started playing with OCaml recently and I've noticed that Merlin's mode menu is almost empty:

image

This doesn't help with the feature discovery.

@erikmd
Copy link
Contributor

erikmd commented Aug 31, 2023

@Chris00 to follow-up your first post:

Merlin shadows C-c C-r (bound in Tuareg and Caml-mode to ...-eval-region). What do you think of binding merlin-error-check to C-c M-r?

For C-c M-r: why not, but that's less convenient to type;

Actually I'd like to suggest binding C-c C-v to this function (merlin-error-check), because this keybinding is unbound in Tuareg/Merlin, and is already used in the standard Emacs modes for Python (to check the ambient script using linters):

@erikmd
Copy link
Contributor

erikmd commented Aug 31, 2023

Dear all, I believe the discussion led to good solutions (requiring few unbindings, for the sake of backward-compatibility),
so I'm about to prepare two small PRs (for Tuareg and Merlin) that perform the various keybinding enhancements that were suggested in these two issues:

@erikmd
Copy link
Contributor

erikmd commented Aug 31, 2023

Here is an (exhaustive) list of the changes that the PRs involve:

Tuareg changes (issue ocaml/tuareg#273 | PR ocaml/tuareg#309, ocaml/caml-mode#18)

Old binding (to remove) Function name Current binding (already existing or new) Comment Author
C-c ? tuareg-interactive-next-error-source C-c ` (existing) Emacs expects ? and C-h to be unbound in prefix maps (for self-documenting purpose, e.g., try C-x ?) Suggested by @mattiase
C-c C-h caml-help C-h . (new) same binding as Eglot Suggested by @catern and @monnier

Merlin changes (issue #1386 | PR #1668)

Old binding (to remove) Function name Current binding (already existing or new) Comment Author
C-c C-r merlin-error-check C-c C-v (new) to avoid shadowing Tuareg's C-c C-r = tuareg-eval-region, and C-c C-v is similar to python.el's and elpy's check commands Suggested by @Chris00 and @erikmd
C-c t (cf. the wiki) merlin-type-expr C-u C-c C-t (new) the binding was documented but not available actually Suggested by @Chris00
C-c C-d merlin-destruct C-c M-d (new) to be able to use C-c C-d for a documentation-related function Suggested by @erikmd and @bbatsov
C-c C-d merlin-destruct C-c | (new) to recall the | in pattern matching and provide a simpler keybinding than C-c M-d Suggested by @erikmd
N/A (unbound) merlin-document C-c C-d (remap) sounds better for a documentation-related function Suggested by @mattiase and @bbatsov

Here is a list of possible future changes (more discussion/work is needed):

Old binding (to remove) Function name Current binding (already existing or new) Comment Author
N/A (unbound) merlin-occurrences M-? (existing) may need new xref-related code Suggested by @mattiase and @Chris00
C-c C-l caml-types-show-ident and merlin-locate M-. (existing) may need new xref-related code Suggested by @Chris00
C-c & merlin-pop-stack M-, (existing) may need new xref-related code Suggested by @Chris00

erikmd added a commit to erikmd/merlin that referenced this issue Aug 31, 2023
erikmd added a commit to erikmd/merlin that referenced this issue Aug 31, 2023
erikmd added a commit to erikmd/merlin that referenced this issue Aug 31, 2023
erikmd added a commit to erikmd/caml-mode that referenced this issue Aug 31, 2023
erikmd added a commit to erikmd/caml-mode that referenced this issue Aug 31, 2023
erikmd added a commit to erikmd/merlin that referenced this issue Sep 11, 2023
erikmd added a commit to erikmd/merlin that referenced this issue Sep 19, 2023
erikmd added a commit to erikmd/merlin that referenced this issue Sep 19, 2023
erikmd added a commit to erikmd/merlin that referenced this issue Sep 19, 2023
voodoos added a commit to voodoos/opam-repository that referenced this issue Sep 21, 2023
CHANGES:

Thu Sep 24 18:01:42 CEST 2023

  + merlin binary
    - Improve error messages for missing configuration reader (ocaml/merlin#1669)
    - Fix regression causing crash when using ppxes under Windows (ocaml/merlin#1673)
    - Fix confusion between aliased modules and module types (ocaml/merlin#1676,
      fixes ocaml/merlin#1667)
    - Ignore hidden branches when listing occurrences (ocaml/merlin#1677, fixes ocaml/merlin#1671)
  + editor modes
    - emacs: fix/improve keybindings (ocaml/merlin#1668, fixes ocaml/merlin#1386):
      Unbind <kbd>C-c C-r</kbd> (to avoid shadowing `tuareg-eval-region`)
      and bind <kbd>C-c C-v</kbd> instead to `merlin-error-check`;
      rebind <kbd>C-c C-d</kbd> to `merlin-document`
      and bind <kbd>C-c M-d</kbd> and <kbd>C-c |</kbd> instead to `merlin-destruct`;
      bind <kbd>C-u C-c C-t</kbd> to `merlin-type-expr`.
      See also <ocaml/merlin#1386 (comment)>
    - emacs: remove use of obsolete `defadvice` macro (ocaml/merlin#1675)
voodoos added a commit to voodoos/opam-repository that referenced this issue Sep 21, 2023
CHANGES:

Thu Sep 24 18:01:42 CEST 2023

  + merlin binary
    - Improve error messages for missing configuration reader (ocaml/merlin#1669)
    - Fix regression causing crash when using ppxes under Windows (ocaml/merlin#1673)
    - Fix confusion between aliased modules and module types (ocaml/merlin#1676,
      fixes ocaml/merlin#1667)
    - Ignore hidden branches when listing occurrences (ocaml/merlin#1677, fixes ocaml/merlin#1671)
  + editor modes
    - emacs: fix/improve keybindings (ocaml/merlin#1668, fixes ocaml/merlin#1386):
      Unbind <kbd>C-c C-r</kbd> (to avoid shadowing `tuareg-eval-region`)
      and bind <kbd>C-c C-v</kbd> instead to `merlin-error-check`;
      rebind <kbd>C-c C-d</kbd> to `merlin-document`
      and bind <kbd>C-c M-d</kbd> and <kbd>C-c |</kbd> instead to `merlin-destruct`;
      bind <kbd>C-u C-c C-t</kbd> to `merlin-type-expr`.
      See also <ocaml/merlin#1386 (comment)>
    - emacs: remove use of obsolete `defadvice` macro (ocaml/merlin#1675)
voodoos added a commit to voodoos/opam-repository that referenced this issue Sep 21, 2023
CHANGES:

Thu Sep 24 18:01:42 CEST 2023

  + merlin binary
    - Add support for OCaml 5.1
    - Improve error messages for missing configuration reader (ocaml/merlin#1669)
    - Fix regression causing crash when using ppxes under Windows (ocaml/merlin#1673)
    - Fix confusion between aliased modules and module types (ocaml/merlin#1676,
      fixes ocaml/merlin#1667)
    - Ignore hidden branches when listing occurrences (ocaml/merlin#1677, fixes ocaml/merlin#1671)
  + editor modes
    - emacs: fix/improve keybindings (ocaml/merlin#1668, fixes ocaml/merlin#1386):
      Unbind <kbd>C-c C-r</kbd> (to avoid shadowing `tuareg-eval-region`)
      and bind <kbd>C-c C-v</kbd> instead to `merlin-error-check`;
      rebind <kbd>C-c C-d</kbd> to `merlin-document`
      and bind <kbd>C-c M-d</kbd> and <kbd>C-c |</kbd> instead to `merlin-destruct`;
      bind <kbd>C-u C-c C-t</kbd> to `merlin-type-expr`.
      See also <ocaml/merlin#1386 (comment)>
    - emacs: remove use of obsolete `defadvice` macro (ocaml/merlin#1675)
voodoos added a commit to voodoos/opam-repository that referenced this issue Sep 21, 2023
CHANGES:

Thu Sep 24 18:01:42 CEST 2023

  + merlin binary
    - Improve error messages for missing configuration reader (ocaml/merlin#1669)
    - Fix regression causing crash when using ppxes under Windows (ocaml/merlin#1673)
    - Fix confusion between aliased modules and module types (ocaml/merlin#1676,
      fixes ocaml/merlin#1667)
    - Ignore hidden branches when listing occurrences (ocaml/merlin#1677, fixes ocaml/merlin#1671)
  + editor modes
    - emacs: fix/improve keybindings (ocaml/merlin#1668, fixes ocaml/merlin#1386):
      Unbind <kbd>C-c C-r</kbd> (to avoid shadowing `tuareg-eval-region`)
      and bind <kbd>C-c C-v</kbd> instead to `merlin-error-check`;
      rebind <kbd>C-c C-d</kbd> to `merlin-document`
      and bind <kbd>C-c M-d</kbd> and <kbd>C-c |</kbd> instead to `merlin-destruct`;
      bind <kbd>C-u C-c C-t</kbd> to `merlin-type-expr`.
      See also <ocaml/merlin#1386 (comment)>
    - emacs: remove use of obsolete `defadvice` macro (ocaml/merlin#1675)
erikmd added a commit to erikmd/caml-mode that referenced this issue Oct 3, 2023
tarsius pushed a commit to emacsmirror/merlin that referenced this issue Oct 18, 2023
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
CHANGES:

Thu Sep 24 18:01:42 CEST 2023

  + merlin binary
    - Improve error messages for missing configuration reader (ocaml/merlin#1669)
    - Fix regression causing crash when using ppxes under Windows (ocaml/merlin#1673)
    - Fix confusion between aliased modules and module types (ocaml/merlin#1676,
      fixes ocaml/merlin#1667)
    - Ignore hidden branches when listing occurrences (ocaml/merlin#1677, fixes ocaml/merlin#1671)
  + editor modes
    - emacs: fix/improve keybindings (ocaml/merlin#1668, fixes ocaml/merlin#1386):
      Unbind <kbd>C-c C-r</kbd> (to avoid shadowing `tuareg-eval-region`)
      and bind <kbd>C-c C-v</kbd> instead to `merlin-error-check`;
      rebind <kbd>C-c C-d</kbd> to `merlin-document`
      and bind <kbd>C-c M-d</kbd> and <kbd>C-c |</kbd> instead to `merlin-destruct`;
      bind <kbd>C-u C-c C-t</kbd> to `merlin-type-expr`.
      See also <ocaml/merlin#1386 (comment)>
    - emacs: remove use of obsolete `defadvice` macro (ocaml/merlin#1675)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment