search

ocaml / tuareg

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

Incorrect highlighting of 'let' with show-paren-mode #26

Open jberdine opened 10 years ago

jberdine commented 10 years ago

With emacs 24.4 and tuareg 2.0.8, enabling show-paren-mode results in somewhat strange highlighting. For a buffer containing only the code:

let f () =
  let x = 0 in
  ()

putting the point at in correctly highlights the matching let, but putting the point at that let highlights it as unmatched. For reference, the behavior of forward-sexp and backward-sexp is consistent, moving from in to let, but not back. The highlighting of parens, brackets, etc. as well as begin and end seems to work as expected.

monnier commented 10 years ago

With emacs 24.4 and tuareg 2.0.8, enabling show-paren-mode results in somewhat strange highlighting. For a buffer containing only the code:

let f () =
  let x = 0 in
  ()

putting the point at in correctly highlights the matching let, but putting the point at that let highlights it as unmatched. For reference, the behavior of forward-sexp and backward-sexp is consistent, moving from in to let, but not back. The highlighting of parens, brackets, etc. as well as begin and end seems to work as expected.

There is a philosophical question lurking here: is "in" the closing element corresponding to the "let" opener? The usual grammar says that it isn't (that the "let" has no matching ender, it just extends "as far as possible").

When you're on the "in" the "let" is highlighted because of smie-blink-matching-inners. But when you're on the "let", the corresponding behavior (which would be to highlight all the elements "at the same level") is not implemented (and maybe not always desirable either, since on an "if" it would end up highlighting all the corresponding else/elsif, which could be too much for some user's taste).

Similarly, C-M-b from right after the "in" will make you jump back to "let", but if you jump forward from right before the "let" you won't stop at "in" but instead go to the end of the whole let expression.

C-M-f and C-M-b normally jump over "a complete subexpression", so jumping from "in" back to "let" is arguably incorrect. I've made the code do that because I find it very useful in practice (much more useful than signaling an error saying that there's no subexpression over which to jump).

    Stefan
jberdine commented 10 years ago

With emacs 24.4 and tuareg 2.0.8, enabling show-paren-mode results in somewhat strange highlighting. For a buffer containing only the code:

let f () =
  let x = 0 in
  ()

putting the point at in correctly highlights the matching let, but putting the point at that let highlights it as unmatched. For reference, the behavior of forward-sexp and backward-sexp is consistent, moving from in to let, but not back. The highlighting of parens, brackets, etc. as well as begin and end seems to work as expected.

There is a philosophical question lurking here: is "in" the closing element corresponding to the "let" opener? The usual grammar says that it isn't (that the "let" has no matching ender, it just extends "as far as possible").

When you're on the "in" the "let" is highlighted because of smie-blink-matching-inners. But when you're on the "let", the corresponding behavior (which would be to highlight all the elements "at the same level") is not implemented (and maybe not always desirable either, since on an "if" it would end up highlighting all the corresponding else/elsif, which could be too much for some user's taste).

Similarly, C-M-b from right after the "in" will make you jump back to "let", but if you jump forward from right before the "let" you won't stop at "in" but instead go to the end of the whole let expression.

C-M-f and C-M-b normally jump over "a complete subexpression", so jumping from "in" back to "let" is arguably incorrect. I've made the code do that because I find it very useful in practice (much more useful than signaling an error saying that there's no subexpression over which to jump).

Thanks for the explanation Stefan. I agree that jumping from "in" back to "let" is very useful. And, not to get into a philosophical debate, but I would expect that jumping from "let" to "in" is also. I find the forward jumping for delimiters very useful.

But even if forward jumping from "let" to "in" is not supported, I would argue that the current behavior of highlighting "let" as an unmatched delimiter is undesirable. It would be fine if there was no highlighting at all, but highlighting it in "show-paren-mismatch-face" is distracting. Also, this highlighting misleadingly gives the impression that there is some way for "let" to be matched, but isn't due to some syntactic mistake.

For reference, "fun", "if", "match", and "try" are in the same situation.

Also, since ocaml uses distinct "else" and "if" instead of a combined "elsif" keyword, wouldn't there be at most two matches for each "if"? It is a matter of personal taste, but just to offer a data point: if there was the option, I would enable highlighting the "then" and "else" matching an "if".

Cheers, Josh

monnier commented 10 years ago

There's no doubt that jumping from "let" to "in" would be useful. But C-M-f can't do that because it already does something else (i.e. jump to the end of the let expression). So we need a new function that jumps to the next sub-element of the current AST node.

And sorry for overlooking the fact that "let" gets highlighted as unmatched; that's indeed an error. Could you report it via "M-x report-emacs-bug" since it's a bug in smie.el rather than in Tuareg?

As for highlighting the "inners" (such as "else", "in", ...) I suggest you file another M-x report-emacs-bug requesting this feature. This will require implementing the new function mentioned before to jump from "let" to "in".

jberdine commented 10 years ago

And sorry for overlooking the fact that "let" gets highlighted as unmatched; that's indeed an error. Could you report it via "M-x report-emacs-bug" since it's a bug in smie.el rather than in Tuareg?

Done, see http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19079.

As for highlighting the "inners" (such as "else", "in", ...) I suggest you file another M-x report-emacs-bug requesting this feature. This will require implementing the new function mentioned before to jump from "let" to "in".

Done, see http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19080.