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

Better handling of \\) in nested environments #2311

Closed
xsrvmy opened this issue Jan 29, 2022 · 12 comments
Closed

Better handling of \\) in nested environments #2311

xsrvmy opened this issue Jan 29, 2022 · 12 comments

Comments

@xsrvmy
Copy link

xsrvmy commented Jan 29, 2022

The current behaviour of \\) is to group the \ with \). Although this is not technically correct, it is actually the desirable behaviour since it prevents the rest of the document from being rehighlighted/reconcealed when entering commands in math mode.
However consider the following:

\(\begin{bmatrix}
a\\b\\)
\end{bmatrix}\)

Here \\) is intended to insert a new line. However this instead breaks syntax highlighting.

I am proposing the following behaviour:

  • If \\) appears in the top-level of an inline math expression, highlight it as \ and \), as using \\ in inline math is very rare, and the user's cursor is likely between the two \'s.
  • If \\) appears in an environment nested inside inline math, highlight it as \\ and ).
    (Note: I don't know how vim's highlighter does tokenizing/parsing, so if this is not possible please close the issue.)
lervag added a commit that referenced this issue Jan 29, 2022
@lervag
Copy link
Owner

lervag commented Jan 29, 2022

I think this is fixed now, please test.

@lervag lervag closed this as completed Jan 29, 2022
@xsrvmy
Copy link
Author

xsrvmy commented Jan 29, 2022

Actually the old behaviour is arguably better for practical purposes even though it is technically not correctly parsing the latex. The new behaviour makes the rest of the document's syntax highlight change if you type a command at the end of inline math.

What I was requesting is if it is possible for this new behaviour to only apply when it is nested inside another environment.

@lervag
Copy link
Owner

lervag commented Jan 30, 2022

I'm sorry, but I really don't understand what you are trying to say. \\) is not a valid LaTeX command - it is \\ followed by ), right? Can you please provide some examples that highlight what you mean?

@lervag
Copy link
Owner

lervag commented Jan 30, 2022

Actually the old behaviour is arguably better for practical purposes even though it is technically not correctly parsing the latex.

I either don't understand or don't agree. I believe the syntax highlighting should be technically correct as long as that is possible, and I believe my commit fixed an issue where your example ended up highlighted wrong because the syntax rules were technically incorrect.

The new behaviour makes the rest of the document's syntax highlight change if you type a command at the end of inline math.

I don't understand. Can you show an example and explain further?

@xsrvmy
Copy link
Author

xsrvmy commented Jan 31, 2022

Basically, I use a snippet for entering and leaving math mode, which leaves me with \(|\) where | is the cursor.
So when I am typing something like \(f \colon X \to Y\), the intermediate result will look like this: \(f \|\), and here \\) would not leave math mode and cause everything up to the next \) to change to math mode. This causes a localized flashing effect on the screen due to the change in syntax highlighting. If conceal is being used, it will also mean that the next \( is not concealed, which causes and entire line to shift.

I actually over estimated the impact of this change because I used $...$ before and for those adding a backslash at the end of the inline math would cause the rest of the document to toggle math mode, which flashes a much bigger area of the screen and completely breaks conceal.

To be fair this is an issue with basically any programming language that has strings anyways. But most languages don't allow regular strings to span more than one line so the effect is more localized.

@lervag
Copy link
Owner

lervag commented Jan 31, 2022

Basically, I use a snippet for entering and leaving math mode, which leaves me with \(|\) where | is the cursor. So when I am typing something like \(f \colon X \to Y\), the intermediate result will look like this: \(f \|\), and here \\) would not leave math mode and cause everything up to the next \) to change to math mode. This causes a localized flashing effect on the screen due to the change in syntax highlighting.

I actually over estimated the impact of this change because I used $...$ before and for those adding a backslash at the end of the inline math would cause the rest of the document to toggle math mode, which flashes a much bigger area of the screen and completely breaks conceal.

Ah, I see now. Ok. And your initial proposal makes more sense as well.

However, this seems very hard. By design, environments do not necessarily create nested syntax groups. Thus it is hard to separate the nested and top-level situation. And I don't have any other good ideas on how to solve this. One possibility could be to rely on the cursor position, but that's too "hacky" even for me.

I believe the best solution is to select either to have \) match "greedy" or not. Right now it is not greedy, because I just fixed that due to this issue. But I could revert and instead match \) greedily under the argument that it would be the right thing almost always, and it would prevent the flashing you mention.

@clason You tend to have opinions on matters like these - what do you think?

@clason
Copy link
Contributor

clason commented Jan 31, 2022

@clason You tend to have opinions on matters like these - what do you think?

You make it sound like it's a good thing...

I think making this context-sensitive is impossible to get right, since you can \input pretty much anything anywhere. Between the original issue and the updated comment, I'd consider the latter scenario to be the more likely and hence more worth accounting for. (In fact, the original example seemed very contrived to me...)

(Actually, my first thoughts on reading the issue were a) yet another reason to use $...$ over \(...\) and b) why not just change the snippet to \(| \) so that the extra space will prevent collisions? But that is neither here nor there.)

@xsrvmy
Copy link
Author

xsrvmy commented Jan 31, 2022

a) yet another reason to use $...$ over (...)
$...$ is even worse for this specific situation. \$ escapes the ending dollar, and causes every $ in the rest of the document to flip meaning. There is also the issue of $|$ breaking highlighting.

And regarding the extra space, that gets deleted on ci$.

I think the best thing to do probably if detecting nesting is too hacky is to make the behaviour of \\) toggleable somehow. I should note that in the old highlighting there was another issue: when conceal is enabled, \\) would conceal the \\ as a CR,
but this is only really a problem if you enable concealing the current line.

@lervag
Copy link
Owner

lervag commented Jan 31, 2022

You make it sound like it's a good thing...

Haha, yes :)

I think making this context-sensitive is impossible to get right, since you can \input pretty much anything anywhere.

Exactly, that's also my thought.

Between the original issue and the updated comment, I'd consider the latter scenario to be the more likely and hence more worth accounting for. (In fact, the original example seemed very contrived to me...)

Sorry, but this is unclear to me. I proposed to either a) keep the currently implemented "greedy" and "technically correct" implementation, or b) revert to the less greedy variant that would fail in some rare cases. The benefit of b) is that it would more or less avoid the flashing stuff.

Which of a) and b) did you find to be most useful?

a) yet another reason to use $...$ over (...)

$...$ is even worse for this specific situation. \$ escapes the ending dollar, and causes every $ in the rest of the document to flip meaning. There is also the issue of $|$ breaking highlighting.

Yes, I agree with this @xsrvmy - however I don't see a good way to avoid it - except perhaps to not use a snippet/auto-closing-delimiter type plugin. I personally just write $ then $ ... then $ ... $. It does break the highlighting, but I don't really notice (or care) much.

I think the best thing to do probably if detecting nesting is too hacky is to make the behaviour of \\) toggleable somehow.

This is also hard. Not impossible, but then I think it is quite hacky and perhaps better to implement as a user customisation.

I should note that in the old highlighting there was another issue: when conceal is enabled, \\) would conceal the \\ as a CR, but this is only really a problem if you enable concealing the current line.

Ah, yes. It's more of an syntax "artifact" than a separate issue.


One thing I could do is to restrict both $...$ and \(...\) to always stop on an empty line. Thus the syntax errors while editing would not go beyond the current paragraph. I believe this is also syntactically correct, as I don't think these constructs allow paragraph breaks.

@clason
Copy link
Contributor

clason commented Jan 31, 2022

Sorry, but this is unclear to me. I proposed to either a) keep the currently implemented "greedy" and "technically correct" implementation, or b) revert to the less greedy variant that would fail in some rare cases. The benefit of b) is that it would more or less avoid the flashing stuff.

Which of a) and b) did you find to be most useful?

I think making \) greedy but not \\ is probably less intrusive, but I personally don't care either way; changing syntax highlighting while I'm typing is just a fact of live that I'm used to.

One thing I could do is to restrict both $...$ and (...) to always stop on an empty line. Thus the syntax errors while editing would not go beyond the current paragraph. I believe this is also syntactically correct, as I don't think these constructs allow paragraph breaks.

Yes, I'd say anything that allows "short-circuiting" syntax elements (without breaking valid syntax) would be a good idea (also for performance reasons, even if the gains should be insignificant in most situations.)

@xsrvmy
Copy link
Author

xsrvmy commented Jan 31, 2022

changing syntax highlighting while I'm typing is just a fact of live that I'm used to.
Conceal is the bigger problem here actually. I remember this being annoying a long time ago. I just use the imap shortcuts to input unicode these days though.

I believe this is also syntactically correct, as I don't think these constructs allow paragraph breaks.

Unfortunately the following is valid code:

\(
\begin{bmatrix}

\end{bmatrix}
\)

This would also break syntax highlighting while typing when inline math is split over multiple lines (due to long commands or matrices).

@lervag
Copy link
Owner

lervag commented Jan 31, 2022

I believe this is also syntactically correct, as I don't think these constructs allow paragraph breaks.

Unfortunately the following is valid code:

\(
\begin{bmatrix}

\end{bmatrix}
\)

This would also break syntax highlighting while typing when inline math is split over multiple lines (due to long commands or matrices).

Ok, I stand corrected - you can have empty lines in inline math. Although I really don't like it. Annoying, because I liked the idea of short circuiting.

Well then, IMHO, there's nothing more to do. I don't see any way to improve this without also making it somehow worse. Thus I propose to not make any further changes.

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