ocaml / tuareg

Emacs OCaml mode
GNU General Public License v3.0
362 stars 79 forks source link

fix(tuareg.el): Fix and improve Emacs bindings #309

Closed erikmd closed 1 year ago

erikmd commented 1 year ago

(following the discussion in #273)

Cc @Chris00 @monnier

For a quick recap of the changes and their motivation, see also the table from comment:

https://github.com/ocaml/merlin/issues/1386#issuecomment-1701567716

erikmd commented 1 year ago

Hi @monnier!

Actually I tried to be as conservative as possible, and I'm not sure it is needed to shrink even further the keymap!

Indeed for example, I checked that C-x ‘ and C-c ‘ are both bound in latex-mode from AUCTeX:

(let ((test (get-buffer-create "test")))
  (set-buffer test)
  (latex-mode)
  (concat
   (describe-key-briefly (kbd "C-c `"))
   "\n"
   (describe-key-briefly (kbd "C-x `"))))

"C-c ` runs the command TeX-next-error  
C-x ` runs the command TeX-next-error"
monnier commented 1 year ago

Indeed for example, I checked that C-x </code> and <code>C-c are both bound in latex-mode from AUCTeX:

Indeed, AUCTeX is another mode that should use next-error-function rather than rebind C-x ` :-)

erikmd commented 1 year ago

@Chris00 @monnier are you happy with the current version of this PR?

— I agree with your earlier comment about the fact we could even unbind "C-c `" but IMHO it is not critical to keep it (all the more as it might surprise some users that use "C-c `" instead of the generic "C-x `")…

monnier commented 1 year ago

Yes, I already approved the change :-)

erikmd commented 1 year ago

Hi, it seems the merlin counterpart of this PR will be merged soon; what about this one?

Anyway, I'd be happy to know @Chris00's opinion, given the three PRs I opened (summarized in this comment) are basically a follow-up of the wrap-up issue he had opened (https://github.com/ocaml/merlin/issues/1386)