Skip to content

Conversation

@kiryph
Copy link
Contributor

@kiryph kiryph commented Jun 11, 2025

This fixes also an issue for following multi-line notation

\usepackage{
  amsmath,
  tikz
}

Related issues, discussions and PRs:

@kiryph kiryph changed the title Parse options for docucment class and packages Parse options for document class and packages Jun 11, 2025
@kiryph kiryph force-pushed the parse-options-for-docclass-and-pkgs branch from 5b0249d to 9dbcb2b Compare June 12, 2025 08:36
@kiryph kiryph force-pushed the parse-options-for-docclass-and-pkgs branch from 9dbcb2b to 9e73b65 Compare June 12, 2025 08:41
with updated __pprint and test file
@kiryph kiryph force-pushed the parse-options-for-docclass-and-pkgs branch from 9e73b65 to b463a44 Compare June 12, 2025 08:42
@lervag
Copy link
Owner

lervag commented Jun 12, 2025

Thanks! This is very nice! I'm merging now with some adjustments from my side.

lervag added a commit that referenced this pull request Jun 12, 2025
@lervag lervag merged commit b463a44 into lervag:master Jun 12, 2025
1 check failed
@kiryph
Copy link
Contributor Author

kiryph commented Jun 12, 2025

Thanks for merging this addition.

I actually like your adjustments and had similarly changes considered: introduction of a shared function to parse options (documentclass or packages) and also to setup a preamble_joined string before doing all the preamble parsing.

I often try to narrow down the modifications for PRs. But I think a sensible refactoring in the context of a PR which introduces more changes can be better.

@kiryph kiryph deleted the parse-options-for-docclass-and-pkgs branch June 12, 2025 10:04
@lervag
Copy link
Owner

lervag commented Jun 13, 2025

Thanks for merging this addition.

My pleasure - I was very glad to see the contribution as I was not immediately sure how to solve #3180. I think your solution looks good and should work in most cases!

I actually like your adjustments and had similarly changes considered: introduction of a shared function to parse options (documentclass or packages) and also to setup a preamble_joined string before doing all the preamble parsing.

I often try to narrow down the modifications for PRs. But I think a sensible refactoring in the context of a PR which introduces more changes can be better.

Glad you like the adjustments. I have a pragmatic view on things like this. I could have merged and then applied the changes, and I could have made comments and asked you to make those changes. Instead, I just made them myself on top before merging because I felt like it. It also helps me to understand your contribution better, which I think is a useful thing in itself.

@ChillyWall
Copy link

This commit breaks my vimtex. When opening a tex file, I would get this error. It seems that the error is caused by the new parse_packages function.

E5108: Error executing lua: vim/_editor.lua:0: nvim_exec2()[1]..BufReadPost Autocommands for "*": Vim(append):Error executing lua callback: /opt/nvim-linux-x86_64/share/nvim/runtime/filetype.lua:36: nvim_exec2()[1]..BufReadPost Autocommands for "*"..FileType Autocommands for "*"..function <SNR>1_LoadFTPlugin[20]..script /home/chilly/.local/share/nvim/lazy/vimtex/ftplugin/tex.vim[25]..function vimtex#init[7]..<SNR>46_init_state[1]..vimtex#state#init[18]..vimtex#state#class#new[37]..<SNR>56_parse_packages[10]..<SNR>56_parse_optionlist, line 8: Vim(let):E687: Less targets than List items
stack traceback:
	[C]: in function '_with'
	/opt/nvim-linux-x86_64/share/nvim/runtime/filetype.lua:35: in function </opt/nvim-linux-x86_64/share/nvim/runtime/filetype.lua:10>

@kiryph
Copy link
Contributor Author

kiryph commented Jun 25, 2025

@ChillyWall Can you post the preamble of the document you are opening?

@ChillyWall
Copy link

@kiryph I'm using pandoc with template Eisvogel. And this is my preamble:

%%
% This is the Eisvogel pandoc LaTeX template.
%
% For usage information and examples visit the official GitHub page:
% https://github.com/Wandmalfarbe/pandoc-latex-template
%%
% Options for packages loaded elsewhere
\PassOptionsToPackage{unicode}{hyperref}
\PassOptionsToPackage{hyphens}{url}
\PassOptionsToPackage{dvipsnames,svgnames,x11names,table}{xcolor}
\PassOptionsToPackage{space}{xeCJK}
\documentclass[
  paper=a4,
  ,captions=tableheading
]{scrartcl}
\usepackage{xcolor}
\usepackage[margin=2.5cm,includehead=true,includefoot=true,centering,]{geometry}
\usepackage{amsmath,amssymb}


% add backlinks to footnote references, cf. https://tex.stackexchange.com/questions/302266/make-footnote-clickable-both-ways
\usepackage{footnotebackref}
\setcounter{secnumdepth}{-\maxdimen} % remove section numbering
\usepackage{iftex}
\ifPDFTeX
  \usepackage[T1]{fontenc}
  \usepackage[utf8]{inputenc}
  \usepackage{textcomp} % provide euro and other symbols
\else % if luatex or xetex
  \usepackage{unicode-math} % this also loads fontspec
  \defaultfontfeatures{Scale=MatchLowercase}
  \defaultfontfeatures[\rmfamily]{Ligatures=TeX,Scale=1}
\fi
\usepackage{lmodern}
\ifPDFTeX\else
  % xetex/luatex font selection
  \setmainfont[]{Libertinus Serif}
  \setsansfont[]{Libertinus Sans}
  \setmonofont[]{Libertinus Mono}
  \setmathfont[]{Libertinus Math}
  \ifXeTeX
    \usepackage{xeCJK}
    \setCJKmainfont[]{Noto Serif CJK SC}
    \setCJKsansfont[]{Noto Sans CJK SC}
    \setCJKmonofont[]{Noto Sans Mono CJK SC}
  \fi
  \ifLuaTeX
    \usepackage[]{luatexja-fontspec}
    \setmainjfont[]{Noto Serif CJK SC}
  \fi
\fi
% Use upquote if available, for straight quotes in verbatim environments
\IfFileExists{upquote.sty}{\usepackage{upquote}}{}
\IfFileExists{microtype.sty}{% use microtype if available
  \usepackage[]{microtype}
  \UseMicrotypeSet[protrusion]{basicmath} % disable protrusion for tt fonts
}{}

% Use setspace anyway because we change the default line spacing.
% The spacing is changed early to affect the titlepage and the TOC.
\usepackage{setspace}
\setstretch{1.2}
\makeatletter
\@ifundefined{KOMAClassName}{% if non-KOMA class
  \IfFileExists{parskip.sty}{%
    \usepackage{parskip}
  }{% else
    \setlength{\parindent}{0pt}
    \setlength{\parskip}{6pt plus 2pt minus 1pt}}
}{% if KOMA class
  \KOMAoptions{parskip=half}}
\makeatother
\setlength{\emergencystretch}{3em} % prevent overfull lines
\providecommand{\tightlist}{%
  \setlength{\itemsep}{0pt}\setlength{\parskip}{0pt}}
\usepackage{bookmark}
\IfFileExists{xurl.sty}{\usepackage{xurl}}{} % add URL line breaks if available
\urlstyle{same}
\definecolor{default-linkcolor}{HTML}{A50000}
\definecolor{default-filecolor}{HTML}{A50000}
\definecolor{default-citecolor}{HTML}{4077C0}
\definecolor{default-urlcolor}{HTML}{4077C0}

\hypersetup{
  pdftitle={代数系统},
  hidelinks,
  breaklinks=true,
  pdfcreator={LaTeX via pandoc with the Eisvogel template}}

\title{代数系统}
\author{}
\date{}


%
% for the background color of the title page
%

%
% break urls
%
\PassOptionsToPackage{hyphens}{url}

%
% When using babel or polyglossia with biblatex, loading csquotes is recommended
% to ensure that quoted texts are typeset according to the rules of your main language.
%
\usepackage{csquotes}

%
% captions
%
\definecolor{caption-color}{HTML}{777777}
\usepackage[font={stretch=1.2}, textfont={color=caption-color}, position=top, skip=4mm, labelfont=bf, singlelinecheck=false, justification=raggedright]{caption}
\setcapindent{0em}

%
% blockquote
%
\definecolor{blockquote-border}{RGB}{221,221,221}
\definecolor{blockquote-text}{RGB}{119,119,119}
\usepackage{mdframed}
\newmdenv[rightline=false,bottomline=false,topline=false,linewidth=3pt,linecolor=blockquote-border,skipabove=\parskip]{customblockquote}
\renewenvironment{quote}{\begin{customblockquote}\list{}{\rightmargin=0em\leftmargin=0em}%
\item\relax\color{blockquote-text}\ignorespaces}{\unskip\unskip\endlist\end{customblockquote}}

%
% Source Sans Pro as the default font family
% Source Code Pro for monospace text
%
% 'default' option sets the default
% font family to Source Sans Pro, not \sfdefault.
%
\ifnum 0\ifxetex 1\fi\ifluatex 1\fi=0 % if pdftex
    \usepackage[default]{sourcesanspro}
  \usepackage{sourcecodepro}
  \else % if not pdftex
    \fi

%
% heading color
%
\definecolor{heading-color}{RGB}{40,40,40}
\addtokomafont{section}{\color{heading-color}}
% When using the classes report, scrreprt, book,
% scrbook or memoir, uncomment the following line.
%\addtokomafont{chapter}{\color{heading-color}}

%
% variables for title, author and date
%
\usepackage{titling}
\title{代数系统}
\author{}
\date{}

%
% tables
%

%
% remove paragraph indentation
%
\setlength{\parindent}{0pt}
\setlength{\parskip}{6pt plus 2pt minus 1pt}
\setlength{\emergencystretch}{3em}  % prevent overfull lines

%
%
% Listings
%
%

%
% header and footer
%
\usepackage[headsepline,footsepline]{scrlayer-scrpage}

\newpairofpagestyles{eisvogel-header-footer}{
  \clearpairofpagestyles
  \ihead*{代数系统}
  \chead*{}
  \ohead*{}
  \ifoot*{}
  \cfoot*{}
  \ofoot*{\thepage}
  \addtokomafont{pageheadfoot}{\upshape}
}
\pagestyle{eisvogel-header-footer}

%
% Define watermark
%

@kiryph
Copy link
Contributor Author

kiryph commented Jun 25, 2025

The line in your preamble causing the error is:

\usepackage[font={stretch=1.2}, textfont={color=caption-color}, position=top, skip=4mm, labelfont=bf, singlelinecheck=false, justification=raggedright]{caption}

The current package option parser does not understand nested options in curly braces as possible for the LaTeX package caption.

The package caption has more unconventional option types:

  • starred variants type*= and margin*=
  • adding nested options to options with the syntax += for font, textfont, and labelfont:
\usepackage[
  font={small,stretch=0.80},
  margin={0pt,10pt},
  margin*=80pt,
  font+=it, % add to font option
  textfont+=it,
  labelfont+=it,
]{caption}

I do not know if there are other LaTeX packages which allow even more deeply nested options or if it is sufficient to extend the parser to understand one level of nested options as for caption.

@ChillyWall
Copy link

@kiryph Thanks. After removing the options, vimtex works again.

@kiryph
Copy link
Contributor Author

kiryph commented Jun 25, 2025

I guess for now you can use \captionsetup{...} until the parser has been extended.

@ChillyWall
Copy link

Yes, I have already changed my template.

lervag added a commit that referenced this pull request Jun 25, 2025
@lervag
Copy link
Owner

lervag commented Jun 25, 2025

This commit breaks my vimtex. When opening a tex file, I would get this error. It seems that the error is caused by the new parse_packages function.

Thanks for reporting this! I just pushed a minor fix; it doesn't actually fix the parsing problem, but it avoids the error and instead silently ignores the problem.

@kiryph This is a classical example of how regexes are not really ideal for this kind of parsing. Now, we could work on a better fix here with more complex regexes, but I think perhaps it might be better to consider a more robust approach.

I believe we might want to do something like what I'm doing here:

function! s:input_to_filename(input, root) abort " {{{2
" Assumption: The input matches g:vimtex#re#tex_input, which means it will
" begin with an input line macro, e.g. " \input{...". We need to extract the
" argument part.
let l:i0 = match(a:input, '{') + 1
let [l:i1, l:_] = vimtex#parser#tex#find_closing(l:i0, a:input, 1, '{')
let l:file = strpart(a:input, l:i0, l:i1-l:i0)

Now, it's not that easy, because we need this more robust parsing both to find the full option string, but also to parse within the string. Thus, the outer split(..., ",") is not really good anymore - instead, we need to seek through the string allow braced values that themselves may contain commas.

I wouldn't say it is very hard, but it is somewhat meticulous.

And I think the right way forward is to start generalizing a set of api functions for parsing things from an input string. There are already quite a few such functions, but it's a little bit messy. :\

@lervag
Copy link
Owner

lervag commented Jun 25, 2025

The vimtex#util#texsplit function is clearly relevant here, though.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants