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

Provide a way to add/identify custom listings environments #2745

Closed
GergelyKalmar opened this issue Jul 1, 2023 · 16 comments · Fixed by #2756
Closed

Provide a way to add/identify custom listings environments #2745

GergelyKalmar opened this issue Jul 1, 2023 · 16 comments · Fixed by #2756

Comments

@GergelyKalmar
Copy link

GergelyKalmar commented Jul 1, 2023

Is your feature request related to a problem? Please describe it.
I'm using the listings package with custom environments:

commons.sty

% ...
\RequirePackage{listings}
\lstset{aboveskip=\baselineskip, backgroundcolor=\color{themegray}, tabsize=4, ...}
\lstnewenvironment{code}[1][]{\lstset{#1}}{}
% ...

test.tex

% ...
\begin{code}[language=python]
def test(argument: str) -> str:
    """
    Test function.
    """
    return argument
\end{code}
% ...

Sadly, in test.tex the code is not highlighted as Python, and neither is the code environment suggested when typing \begin{. The lstnewenvironment command is in a separate sty file (installed under ~/texmf/tex/latex/...) but I also tried inlining it in test.tex and it didn't work. Commands are otherwise properly identified and autocompleted from commons.sty, so I suspect the issue is specifically related to lstnewenvironment. I've compiled the tex source and cleared the cache, did not help. Using VimTeX 2.13.

Note that if I change \begin{code} to \begin{lstlisting} the syntax highlighting and autocompletion works as expected.

Describe the solution you'd like
Optimally the lstnewenvironment environments would be automatically picked up, but I would also be fine with a config option where I can set aliases for lstlisting environments.

@lervag
Copy link
Owner

lervag commented Jul 1, 2023

Can you please provide a full minimal example?

@lervag
Copy link
Owner

lervag commented Jul 1, 2023

lstnewenvironment is currently not supported. However, if you use minted, then you will have something similar. I.e., the following should work:

\documentclass{minimal}
\usepackage{minted}
\newminted[code]{python}{opts}

\begin{document}

\begin{code}
def test(argument: str) -> str:
    """
    Test function.
    """
    return argument
\end{code}

\end{document}

@lervag
Copy link
Owner

lervag commented Jul 1, 2023

But it won't work if you move the \newminted command to a commonts.sty. I don't plan to add that type of support either, because it makes things very complicated. This is because the feature relies on parsing every \newminted command found in the document. This is already somewhat expensive when we parse a large project. If we were to also recursively search for such commands in every loaded package, then it will likely have a large performance impact.

@lervag
Copy link
Owner

lervag commented Jul 1, 2023

So: What you want is already possible with minted as long as you keep the definitions in your preamble (or any included file; not a package). I've found minted to be better than lstlistings for these things. So I would kindly ask if you would possibly find this to be a satisfactory solution.

If not, then I could consider the feature request to add support for a similar feature with lstlistings with the \lstnewenvironment command.

@GergelyKalmar
Copy link
Author

That's a very good suggestion, I will check if I can replace listings with minted instead. I do like the interface better too, I will just need to check if I can reproduce our current styles.

@GergelyKalmar
Copy link
Author

GergelyKalmar commented Jul 1, 2023

I've checked, and minted indeed does have a nicer interface and is definitely better in terms of features, however, there are also a few issues that would make it difficult for me to switch. The biggest issue by far would be that it requires -shell-escape, which could lead to vulnerabilities when deployed on a web service. Having a dependency on Pygments is doable but is not very elegant either. I think I'd stay on lstlistings for now.

I was wondering about what you mentioned when it comes to recursively searching in packages – isn't that what you are doing anyways when looking for \newcommand already? Would it be possible to extend that so that you collect multiple things in one pass-through, e.g. \lstnewenvironment commands too?

@lervag
Copy link
Owner

lervag commented Jul 2, 2023

I've checked, and minted indeed does have a nicer interface and is definitely better in terms of features, however, there are also a few issues that would make it difficult for me to switch. The biggest issue by far would be that it requires -shell-escape, which could lead to vulnerabilities when deployed on a web service. Having a dependency on Pygments is doable but is not very elegant either. I think I'd stay on lstlistings for now.

Ok; I'll add the feature for lstlistings, then.

I was wondering about what you mentioned when it comes to recursively searching in packages – isn't that what you are doing anyways when looking for \newcommand already?

For completion purposes, I am searching packages for \newcommands. But this is 1) not recursive, and 2) performed on demand with a caching mechanism.

Would it be possible to extend that so that you collect multiple things in one pass-through, e.g. \lstnewenvironment commands too?

No, I don't think so. I believe this is one of the things where I could have been smarter from the start. I should have indexed projects as they are loaded in a more general manner similar to what modern IDEs do. But instead, I've only searched through files as necessary when I needed it. This does mean there is duplicate work being done, but I think it will be a lot of work to change this.

Let's start by supporting your desired feature without loading from packages. When that works, I may reconsider looking into the packages as well.

@lervag
Copy link
Owner

lervag commented Jul 2, 2023

Do you agree that this is a sensible minimal example?

\documentclass{minimal}
\usepackage{listings}
\lstnewenvironment{code}[1][]{\lstset{#1}}{}

\begin{document}

\begin{code}[language=python]
def test(argument: str) -> str:
    """
    Test function.
    """
    return argument
\end{code}

\end{document}

@GergelyKalmar
Copy link
Author

Yes, having said that, it won't help in my particular case as the new environment is indeed created in a package. The idea is to specify the style of that environment in the package and then have it applied automatically whenever someone uses it.

While I'm not sure how you would feel about it, it would be more useful to me to have an option to set an alias (or to specify a list of environments) that are "code-like" (so that syntax highlighting should be applied). This way it would not matter where the environment is defined, it would work with lstlistings but also any other alias we create. I don't mind if it is not entirely automatic, I can set an option accordingly.

@lervag
Copy link
Owner

lervag commented Jul 6, 2023

Yes, having said that, it won't help in my particular case as the new environment is indeed created in a package. The idea is to specify the style of that environment in the package and then have it applied automatically whenever someone uses it.

No, I can understand that. But I have to start somewhere, and I think it makes sense to start at the most simple stuff.

While I'm not sure how you would feel about it, it would be more useful to me to have an option to set an alias (or to specify a list of environments) that are "code-like" (so that syntax highlighting should be applied). This way it would not matter where the environment is defined, it would work with lstlistings but also any other alias we create. I don't mind if it is not entirely automatic, I can set an option accordingly.

Yes, perhaps we could do that. Something along the same idea as g:vimtex_syntax_custom_cmds. Perhaps g:vimtex_syntax_custom_nested_envs.

But I would prefer to first implement the support for the listings package with the earlier mentioned restriction that we only consider the current "project" and not recurse into packages. After that, I believe it makes sense to either 1) consider to recurse also into the packages, or 2) add a new configuration option as suggested here.

@GergelyKalmar
Copy link
Author

Sure, I leave it for your judgement! Thank you very much for your thoughts.

@lervag
Copy link
Owner

lervag commented Jul 12, 2023

Hmm. I've looked into the listings documentation, and I find that the possibilities here are quite large. In particular, you can define environments with fully predefined options, and you can define environments that take the options themselves. This makes it much harder to properly parse, because the option groups would have to be parsed twice and the syntax rules will depend on both locations.

I believe it may be much easier and perhaps equally practical to allow an option to include nested syntaxes. Perhaps something like the :help g:vimtex_syntax_custom_cmds? E.g., I could implement something like this:

*g:vimtex_syntax_custom_envs*
  A list of environments for which to apply custom styling. This allows e.g. to
  specify nested syntaxes for custom environments with the listings package
  (`\lstnewenvironment`).

  Each environment is expected to be of the following type: >

    \begin{env_name}[optional argument]

    \end{env_name}
<
  It is important to be aware that these customizations will be applied on top
  of the existing syntax rules. These may therefore override both the core
  syntax rules and extensions from syntax packages.

  Each element in the list must be a dictionary with the following keys:

    name ~
      Default: Undefined (REQUIRED)
      The environment to highlight (`env_name`). This is also for defining the
      syntax group names.

    mathmode ~
      Default: |v:false|
      If true, then the environment opens a math region.

    nested ~
      Default: Undefined
      This can be either a string or a dictionary. If it is a string, then it
      will specify the nested syntax to load inside the environment. If it is
      a dictionary, then it is used as a predicate and thus gives more
      flexibility.

      The dictionary uses the target syntax as the key and the "predicate" as
      the value. This predicate is a string that must be contained within the
      optional argument. See below for an example.

    hlgroup ~
      Default: Undefined
      A string that can be used to indicate the target highlight group of the
      command (`\cmdname`).

  The following example creates rules for two environments. The first creates
  the environment `MyMathEnv` that opens a new math environment, whereas the
  second creates a `code` environment that applies nested Python syntax rules
  in the environment region. >vim

    let g:vimtex_syntax_custom_cmds = [
          \ {'name': 'MyMathEnv', 'mathmode': 1},
          \ {'name': 'python_code', 'nested': 'python'},
          \ {'name': 'code', 'nested': {
          \     'python': 'language=python',
          \     'c': 'language=C',
          \     'rust': 'language=rust',
          \   },
          \ },
          \]
<
  Default value: []

This should give a lot of flexibility, and I believe it may solve your use case?

lervag added a commit that referenced this issue Jul 12, 2023
@lervag
Copy link
Owner

lervag commented Jul 12, 2023

I've started implementing this in a separate branch. I've currently added a spec similar to what I proposed above and I've added some test files.

@GergelyKalmar
Copy link
Author

This looks excellent! Yes, I think it would solve my use case nicely without needing the complex parsing in packages. Love it.

lervag added a commit that referenced this issue Jul 12, 2023
lervag added a commit that referenced this issue Jul 12, 2023
Adds option g:vimtex_syntax_custom_envs in the same spirit as the
corresponding g:vimtex_syntax_custom_cmds.

refer: #2745
@lervag
Copy link
Owner

lervag commented Jul 12, 2023

You can start testing in the feature branch. It is not fully complete yet, but the simple string form of the nested option does work.

lervag added a commit that referenced this issue Jul 12, 2023
Adds a new option g:vimtex_syntax_custom_envs that allows to define
syntax rules for custom environments in the same spirit as the
corresponding g:vimtex_syntax_custom_cmds.

refer: #2745
@lervag
Copy link
Owner

lervag commented Jul 12, 2023

Ok; I believe this is fully ready now. Are you comfortable with checking out the branch and testing locally? If so, I would be happy to get feedback before I merge into master. See #2756.

I'll close this issue; let's continue the discussion on the PR thread.

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

Successfully merging a pull request may close this issue.

2 participants