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

Highlight \iffalse … \fi as a comment #1988

Closed
psvenk opened this issue Mar 10, 2021 · 13 comments
Closed

Highlight \iffalse … \fi as a comment #1988

psvenk opened this issue Mar 10, 2021 · 13 comments

Comments

@psvenk
Copy link
Contributor

psvenk commented Mar 10, 2021

Similarly to how c.vim shipped with Vim highlights anything between #if 0 and #endif as a comment, it would be nice if VimTeX highlighted anything between \iffalse and \fi as a comment, given that these play a similar role in TeX and are a useful way to comment out large swaths of TeX source.

In a similar vein, it may be useful to highlight the interior of a comment environment (from the comment package) as a comment.

@lervag
Copy link
Owner

lervag commented Mar 10, 2021

It's not a such a bad idea, I think. I'd like a second opinion, though (perhaps @clason?).

If we did this, then I think \iffalse and \fi should still be highlighted to help distinguish this region.

@clason
Copy link
Contributor

clason commented Mar 10, 2021

Highlighting contents of the comment environment as a comment sounds good to me.

I'm not sure about \iffalse...\fi, as this is assuming that this construct is (ab)used for something specific (that is different from the general intended use as a programming language construct). (Also, this is TeX, not LaTeX, so it should depend on the flavor?)

@psvenk
Copy link
Contributor Author

psvenk commented Mar 10, 2021

Thank you @lervag and @clason for your speedy responses! I would argue that \iffalse\fi is quite similar to #if 0 and #endif, which is very commonly used for something specific in the same way. The answers to this question on TeX - LaTeX Stack Exchange seem to suggest that this practice is quite common even in LaTeX code and particularly in dtx files as meta-comments (because the documentation is all written in comments); for example, geometry.dtx uses it.

Because VimTeX already supports the use of ^^A for this in dtx files, it may be a good idea to support \iffalse as well (and extend it to all (La)TeX files for that matter).

As for highlighting \iffalse and \fi themselves, this is in line with how c.vim does it. Here is a screenshot with :colo default:
image

@j1-lee
Copy link
Contributor

j1-lee commented Mar 10, 2021

I agree with @clason. Although I don't fully understand the nature of \iffalse, according to the Stack Exchange article @psvenk provided, there seem to be other usages of \iffalse (in the answers by egreg and yo').

For example, in latex.ltx, we can find the following part:

\let\if@tempswa\iffalse
\def\loop#1\repeat{\def\iterate{#1\relax\expandafter\iterate\fi}%
  \iterate \let\iterate\relax}

I guess that highlighting the characters between \iffalse and \fi would not be correct.

@psvenk
Copy link
Contributor Author

psvenk commented Mar 10, 2021

@j1-lee This is a good point. What if we only highlighted the contents of \iffalse\fi if each one is the first (or only) token on its line?

I now realize that geometry.dtx has an \iffalse block that spans multiple documentation blocks (that is, there is also package code interspersed between, and we don't want that to be highlighted as a comment). I imagine that highlighting that would be quite complex.

Given the above, I'm not sure whether or not this is too specialized to be included in VimTeX, given that it would entail handling various edge cases. I had just thought that it would be a convenient quality of life addition, but it seems that this might open up a can of worms; however, I would still be happy to implement it if it is deemed beneficial as a feature to include. Another possibility is to use the approach I detailed previously and just exclude dtx files.

@lervag
Copy link
Owner

lervag commented Mar 10, 2021

Highlighting contents of the comment environment as a comment sounds good to me.

I'll look into that!

@clason

I'm not sure about \iffalse...\fi, as this is assuming that this construct is (ab)used for something specific ...

@j1-lee

I agree with @clason. ...

Thanks for pitching in! I really do appreciate it; it is sometimes hard to decide what is a good idea and what is not - often things may seem like both, and we just have to make a choice.

@j1-lee This is a good point. What if we only highlighted the contents of \iffalse\fi if each one is the first (or only) token on its line?
...

Perhaps we could add it as an optional highlight? Something like g:vimtex_syntax_comment_iffalse, which would be set to 0 or v:false by default (to avoid surprising long time users).

@lervag
Copy link
Owner

lervag commented Mar 10, 2021

Note, the following should suffice for adding \iffalse ... \endif \iffalse ... \fi as comments:

syntax region texComment matchgroup=texCmd start="\\iffalse" end="\\fi"

lervag added a commit that referenced this issue Mar 10, 2021
@psvenk
Copy link
Contributor Author

psvenk commented Mar 10, 2021

Thank you @lervag for your thoughtful comments and for 3809599. A few comments:

  • I think you made a typo (I made the same typo earlier and just now retroactively fixed it); \endif should be \fi.

  • It may be a good idea to enable folding, both for comment environments (3809599) and for \iffalse ... \endif, in line with how many other environments are folded and how multi-line comments are folded for other languages (including #if 0 ... #endif in C).

  • I think it may still be a good idea to put restrictions in place on the \iffalse code. I tested your proposed code (substituting \\fi for \\endif), and the results are (arguably) tolerable but suboptimal on geometry.dtx:

    Screenshot

    image

  • On a slightly unrelated note, I noticed that the ^^M detection for dtx files currently does not work, presumably because texSpecialChar is defined after texComment in syntax/core.vim, so ^^M is highlighted as a texSpecialChar and the remainder of the line is not highlighted as a comment but as normal LaTeX.

@lervag
Copy link
Owner

lervag commented Mar 11, 2021

  • I think you made a typo (I made the same typo earlier and just now retroactively fixed it); \endif should be \fi.

Thanks, fixed :)

  • It may be a good idea to enable folding, both for comment environments (3809599) and for \iffalse ... \endif, in line with how many other environments are folded and how multi-line comments are folded for other languages (including #if 0 ... #endif in C).

I think this is a completely new and separate thread. And I'm not fully sure what you mean: folding is enabled, especially for the comment environment (as environments are folded in general). Conditionals are also folded, but \iffalse is not in the default regex. See :help vimtex_indent_conditionals. If you think \iffalse should be folded by default, please raise a new issue - I might agree, but I have not thought about it yet.

  • I think it may still be a good idea to put restrictions in place on the \iffalse code. I tested your proposed code (substituting \\fi for \\endif), and the results are (arguably) tolerable but suboptimal on geometry.dtx:

Ah, yes - in this particular case, there is also the problem of nested conditionals: \iffalse -> \ifcase -> \ifx -> \fi ...; my initial proposal fails because it ends on the first \fi. This can be fixed by use of nested syntax groups. Something simple will probably suffice, e.g. a texCommentContainedConds groups that is only matched inside texComment and that contains transparent regions between \if\w\+ -> \fi.

If I add an optional \iffalse as comment-feature, I will definitely apply my above suggested improvement.

So: to do or not to do add this feature? It seems you are already interested in this, and I would guess it could be of interest to more people. So I'll raise a "why not"?

  • On a slightly unrelated note, I noticed that the ^^M detection for dtx files currently does not work, presumably because texSpecialChar is defined after texComment in syntax/core.vim, so ^^M is highlighted as a texSpecialChar and the remainder of the line is not highlighted as a comment but as normal LaTeX.

Ah, good point; I never tested and so never noticed that. My fault.

@psvenk
Copy link
Contributor Author

psvenk commented Mar 11, 2021

And I'm not fully sure what you mean: folding is enabled, especially for the comment environment (as environments are folded in general).

I realized that I had not even enabled folding in VimTeX! I had grown used to folding in other languages and just assumed that it was already enabled in VimTeX when I had actually been living without folds. I enabled folding and the comment environment is folded as expected, in addition to VimTeX's superb folding in general.

Ah, yes - in this particular case, there is also the problem of nested conditionals: \iffalse -> \ifcase -> \ifx -> \fi ...; my initial proposal fails because it ends on the first \fi. This can be fixed by use of nested syntax groups. Something simple will probably suffice, e.g. a texCommentContainedConds groups that is only matched inside texComment and that contains transparent regions between \if\w\+ -> \fi.

This sounds like a good way to do it. Do you have any idea how to mitigate the additional problem of intervening code in dtx files? (For example, in geometry.dtx, the corresponding \fi for the \iffalse is on line 123, but there is some package code before that line which should be highlighted normally.)

  • On a slightly unrelated note, I noticed that the ^^M detection for dtx files currently does not work […]

Ah, good point; I never tested and so never noticed that. My fault.

Thanks!

lervag added a commit that referenced this issue May 30, 2021
@lervag
Copy link
Owner

lervag commented May 30, 2021

Sorry for the delay here! It's been very busy the last month.

I just pushed a change that I think might work well in almost every case, so that the option is not necessary. Let me know what you think, and if I'm wrong, I'll be happy to add a switch for this (with default set to off).

@lervag lervag closed this as completed May 30, 2021
@psvenk
Copy link
Contributor Author

psvenk commented Jun 1, 2021

Yes, this fix works well for me. Thank you @lervag!

@lervag
Copy link
Owner

lervag commented Jun 1, 2021

Great :)

julio-b added a commit to julio-b/vimtex that referenced this issue Feb 1, 2022
The contents after \else should not be highlighted as a comment
ref: lervag#1988
julio-b added a commit to julio-b/vimtex that referenced this issue Feb 1, 2022
Highlight the \else section of \iftrue command as a comment
ref: lervag#1988
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

4 participants