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

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

Open
jberdine opened this issue Nov 13, 2014 · 4 comments
Open

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

jberdine opened this issue Nov 13, 2014 · 4 comments

Comments

@jberdine
Copy link

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
Copy link
Contributor

monnier commented Nov 13, 2014

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
Copy link
Author

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
Copy link
Contributor

monnier commented Nov 14, 2014

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants