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

inner environment text-object fails on { being the first char after \begin{} #2160

Closed
habamax opened this issue Sep 5, 2021 · 21 comments
Closed
Labels

Comments

@habamax
Copy link
Contributor

habamax commented Sep 5, 2021

Description

No way to visually select or delete inner environment with vie, die for the code block of the json source.

Steps to reproduce

  1. vim -u minimal.vim minimal.tex
  2. position cursor on "contacts"
  3. vie

minimal.tex

\documentclass{minimal}
\begin{document}
\begin{minted}
    {
        "contacts": [
          {
            "source_id": "mandatory"
          }
        ]
    }
\end{minted}
\end{document}

Expected behavior

Contents of the minted environment is visually selected:
image

Actual behavior

A single char is selected at the end of the environment:
image

Do you use a latexmkrc file?

No

VimtexInfo

System info
  OS: Microsoft Windows 10 Pro (10.0.19041 N/A Build 19041)
  Vim version: VIM 8.2 (1-3399)
  Has clientserver: true
  Servername: GVIM

VimTeX project: minimal
  base: minimal.tex
  root: C:\Users\maksim.kim
  tex: C:\Users\maksim.kim\minimal.tex
  main parser: current file
  document class: 
  compiler: latexmk
    engine: -lualatex
    options:
      -shell-escape
      -verbose
      -file-line-error
      -synctex=1
      -interaction=nonstopmode
    build_dir: minimal
    callback: 1
    continuous: 1
    executable: latexmk
  viewer: General
  qf method: LaTeX logfile
@habamax habamax added the bug label Sep 5, 2021
@habamax habamax changed the title inner environment text-object fails on { being the first char after begin inner environment text-object fails on { being the first char after \begin{} Sep 5, 2021
@habamax
Copy link
Contributor Author

habamax commented Sep 5, 2021

It didn't work for me with crowded minted environment:
image

And only if I add non { char (not space) before { it starts recognizing inner environment:
image

@habamax
Copy link
Contributor Author

habamax commented Sep 5, 2021

And vae, dae works no matter what.

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

vae and dae are simple. The problem here is that the initial { is very confusing. There is no "generic" manner to parse TeX reliably, so how do we know it is not part of the \begin{...}[..]{..} command?

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

I think we need to make some heuristic, but I'm not sure if it is safe to do so.

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

You would get the same problem with

\begin{minted}
    [
        "contacts": [
          {
            "source_id": "mandatory"
          }
        ]
    ]
\end{minted}

The point, as hinted, is that VimTeX parses \begin{minted}{...} as the beginning of the environment. The parser allows separating white space, including newlines. And this really is allowed LaTeX. Notice that the following will compile perfectly fine:

\documentclass{article}
\usepackage{minted}
\begin{document}

\begin{minted}
  [
    numbersep=5mm,
    linenos=true,
    fontsize=\footnotesize,
    breaklines
  ]
  {
    json
  }
  {
    "contacts": [
      {
        "source_id": "mandatory"
      }
    ]
  }
\end{minted}
\end{document}

My point is only to show that it really is hard to properly parse this stuff. And I don't know if I'm able to really find a good way to make it work reliably.

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

In my last example: I think we can both agree that the inner part of the environment starts at {\n "contacts" (\n indicates newline). But the current parser is greedy and "gobbles" up the last { ... } also as an argument for \begin{minted}, and so it puts the start of the inner area at the final } before \end{minted}.

So, at least the issue is now fully understood and explained. It remains to see if it is possible to suggest any solution. I'm afraid it is going to be hard.. :\

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

Perhaps a workaround is to add an empty line first? Or a json comment?

@habamax
Copy link
Contributor Author

habamax commented Sep 5, 2021

I can totally understand it is not a trivial thing to do, indeed.

  1. Empty line doesn't make any difference
  2. json comment might be a workaround but for now I just vaekoj :)

@habamax
Copy link
Contributor Author

habamax commented Sep 5, 2021

I am thinking of non-greedy {}[]{} detection, could you point me to the place where it is defined?

@habamax
Copy link
Contributor Author

habamax commented Sep 5, 2021

One additional thing: cac on \begin deletes all the way up to \end :)

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

Yes; for the exact same reason.

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

It is more or less impossible to write a perfect parser here, I believe. To do so, we need to know the exact signature of each command. But we don't. :\

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

I am thinking of non-greedy {}[]{} detection, could you point me to the place where it is defined?

In some cases, non-greedy would be perfect. In others, it would not be good and you would have to adjust.

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

The text object function is here:

function! vimtex#text_obj#delimited(is_inner, mode, type) abort " {{{1
let l:object = {}
let l:prev_object = {}
let l:pos_save = vimtex#pos#get_cursor()
let l:startpos = getpos("'>")
" Get the delimited text object positions
for l:count in range(v:count1)
if !empty(l:object)
let l:pos_next = vimtex#pos#prev(
\ a:is_inner ? l:object.open : l:object.pos_start)
if a:mode
let l:startpos = l:pos_next
else
call vimtex#pos#set_cursor(l:pos_next)
endif
endif
if a:mode
let l:object = s:get_sel_delimited_visual(a:is_inner, a:type, l:startpos)
else
let [l:open, l:close] = vimtex#delim#get_surrounding(a:type)
let l:object = empty(l:open)
\ ? {} : s:get_sel_delimited(l:open, l:close, a:is_inner)
endif
if empty(l:object)
if !empty(l:prev_object) && g:vimtex_text_obj_variant !=# 'targets'
let l:object = l:prev_object
break
endif
if a:mode
normal! gv
else
call vimtex#pos#set_cursor(l:pos_save)
endif
return
endif
let l:prev_object = l:object
endfor
" Handle empty inner objects
if vimtex#pos#smaller(l:object.pos_end, l:object.pos_start)
if v:operator ==# 'y' && !a:mode
return
endif
if index(['c', 'd'], v:operator) >= 0
call vimtex#pos#set_cursor(l:object.pos_start)
normal! ix
endif
let l:object.pos_end = l:object.pos_start
endif
" Apply selection
execute 'normal!' l:object.select_mode
call vimtex#pos#set_cursor(l:object.pos_start)
normal! o
call vimtex#pos#set_cursor(l:object.pos_end)
endfunction
" }}}1

Here the inner object is narrowed:

" Adjust the borders
if a:is_inner
if has_key(a:open, 'env_cmd') && !empty(a:open.env_cmd)
let l1 = a:open.env_cmd.pos_end.lnum
let c1 = a:open.env_cmd.pos_end.cnum+1

The parser for environment delimiters is here:

function! s:parser_env(match, lnum, cnum, ...) abort " {{{1
let result = {}
let result.type = 'env'
let result.name = matchstr(a:match, '{\zs\k*\ze\*\?}')
let result.starred = match(a:match, '\*}$') > 0
let result.side = a:match =~# '\\begin' ? 'open' : 'close'
let result.is_open = result.side ==# 'open'
let result.get_matching = function('s:get_matching_env')
let result.gms_flags = result.is_open ? 'nW' : 'bnW'
let result.gms_stopline = result.is_open
\ ? line('.') + g:vimtex_delim_stopline
\ : max([1, line('.') - g:vimtex_delim_stopline])
if result.is_open
let result.env_cmd = vimtex#cmd#get_at(a:lnum, a:cnum)
endif
let result.corr = result.is_open
\ ? substitute(a:match, 'begin', 'end', '')
\ : substitute(a:match, 'end', 'begin', '')
let result.re = {
\ 'open' : '\m\\begin\s*{\w\+\*\?}',
\ 'close' : '\m\\end\s*{\w\+\*\?}',
\}
let result.re.this = result.is_open ? result.re.open : result.re.close
let result.re.corr = result.is_open ? result.re.close : result.re.open
return result
endfunction
" }}}1

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

This is the outer delimiter parser, the starting point of the parser:

function! s:get_delim(opts) abort " {{{1
"
" Arguments:
" opts = {
" 'direction' : next
" prev
" current
" 'type' : env_tex
" env_math
" env_all
" delim_tex
" delim_math
" delim_modq_math (possibly modified math delimiter)
" delim_mod_math (modified math delimiter)
" delim_all
" all
" 'side' : open
" close
" both
" 'syn_exclude' : Don't match in given syntax
" }
"
" Returns:
" delim = {
" type : env | delim
" side : open | close
" name : name of environment [only for type env]
" lnum : number
" cnum : number
" match : unparsed matched delimiter
" corr : corresponding delimiter
" re : {
" open : regexp for the opening part
" close : regexp for the closing part
" }
" remove : method to remove the delimiter
" }
"
let l:save_pos = vimtex#pos#get_cursor()
let l:re = g:vimtex#delim#re[a:opts.type][a:opts.side]
while 1
let [l:lnum, l:cnum] = a:opts.direction ==# 'next'
\ ? searchpos(l:re, 'cnW', line('.') + g:vimtex_delim_stopline)
\ : a:opts.direction ==# 'prev'
\ ? searchpos(l:re, 'bcnW', max([line('.') - g:vimtex_delim_stopline, 1]))
\ : searchpos(l:re, 'bcnW', line('.'))
if l:lnum == 0 | break | endif
if has_key(a:opts, 'syn_exclude')
\ && vimtex#syntax#in(a:opts.syn_exclude, l:lnum, l:cnum)
call vimtex#pos#set_cursor(vimtex#pos#prev(l:lnum, l:cnum))
continue
endif
break
endwhile
call vimtex#pos#set_cursor(l:save_pos)
let l:match = matchstr(getline(l:lnum), '^' . l:re, l:cnum-1)
if a:opts.direction ==# 'current'
\ && l:cnum + strlen(l:match) + (mode() ==# 'i' ? 1 : 0) <= col('.')
let l:match = ''
let l:lnum = 0
let l:cnum = 0
endif
let l:result = {
\ 'type' : '',
\ 'lnum' : l:lnum,
\ 'cnum' : l:cnum,
\ 'match' : l:match,
\ 'remove' : function('s:delim_remove'),
\}
for l:type in s:types
if l:match =~# '^' . l:type.re
let l:result = extend(
\ l:type.parser(l:match, l:lnum, l:cnum,
\ a:opts.side, a:opts.type, a:opts.direction),
\ l:result, 'keep')
break
endif
endfor
return empty(l:result.type) ? {} : l:result
endfunction
" }}}1

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

I have to admit I'm tempted to not spend more time on this, as I can't see any clear strategy for fixing this that won't break other functionality. :\

@habamax
Copy link
Contributor Author

habamax commented Sep 5, 2021

I have to admit I'm tempted to not spend more time on this, as I can't see any clear strategy for fixing this that won't break other functionality. :\

You're probably right. I can only think of a newline-like ^\s*$ separator between parameters and body text.

Anyway, I can agree there is no clear solution to the problem.

@habamax
Copy link
Contributor Author

habamax commented Sep 5, 2021

As a workaround I can use a <C-k><space><space> added to the beginning of the env body.

@lervag
Copy link
Owner

lervag commented Sep 5, 2021

So, just to be clear: I believe the current implementation works "as expected" most of the time, and that your specific example is simply a good example of where it does not work as expected.

I can only think of a newline-like ^\s*$ separator between parameters and body text.

I agree with that, actually, so I'll look into it.

lervag added a commit that referenced this issue Sep 5, 2021
@lervag
Copy link
Owner

lervag commented Sep 5, 2021

I can only think of a newline-like ^\s*$ separator between parameters and body text.

I agree; just pushed a fix for that.

As a workaround I can use a <C-k><space><space> added to the beginning of the env body.

Ah, yes, that seems like an ok workarond.

For other readers: <c-k><space><space> in insert mode will create an unbreakable space, a different type of space. It is not matched with the regex pattern \s, which is why it works as a workaround.

@lervag lervag closed this as completed Sep 5, 2021
@habamax
Copy link
Contributor Author

habamax commented Sep 6, 2021

Thx, empty line separator looks like a good solution for this.

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

No branches or pull requests

2 participants