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

Problematic syntax ftplugin files for folding #16083

Open
avidseeker opened this issue Nov 19, 2024 · 10 comments
Open

Problematic syntax ftplugin files for folding #16083

avidseeker opened this issue Nov 19, 2024 · 10 comments
Labels

Comments

@avidseeker
Copy link
Contributor

Steps to reproduce

Moving from neovim/neovim#31233

mwe.rst:

================
Document Heading
================

Heading
=======

Sub-heading
-----------

Paragraphs are separated 
by a blank line.

and vim --clean -u vimrc +'set fdm' mwe.rst where vimrc is

set nocompatible
let g:rst_fold_enabled = 1      
let g:markdown_folding = 1      ":set fdm returns manual
"let g:javaScript_fold = 1       ":set fdm returns syntax
filetype plugin on
syntax on

Everything works as expected. Markdown folding doesn't interfere with rst
folding. However, adding any of these lines

let g:javaScript_fold = 1       ":set fdm returns syntax
let g:php_folding = 1       ":set fdm returns syntax

and now rst folding no longer works. I only managed to find php and javascript, but there could be other filetype plugins having the same issue, too.

Expected behaviour

.

Version of Vim

9.1

Environment

Linux

Logs and stack traces

No response

@avidseeker avidseeker added the bug label Nov 19, 2024
@zeertzjq
Copy link
Member

This is a problem with the syntax files, not filetype plugins.

@chrisbra
Copy link
Member

let g:javaScript_fold = 1 ":set fdm returns syntax
let g:php_folding = 1 ":set fdm returns syntax

If I understand correctly, this will just enable syntax-based folding for the embedded javascript/php language. Why do you expect this to enable folding of your RST document?

@Konfekt
Copy link
Contributor

Konfekt commented Nov 19, 2024

Why do you expect this to enable folding of your RST document?

As these were added after the line coming before

let g:rst_fold_enabled = 1

which entails

" /usr/local/share/vim/vim91/ftplugin/rst.vim (lines 37-44)
if g:rst_fold_enabled != 0 && has('patch-7.3.867')  " Introduced the TextChanged event.
  setlocal foldmethod=expr
  setlocal foldexpr=RstFold#GetRstFold()
  setlocal foldtext=RstFold#GetRstFoldText()
  augroup RstFold
    autocmd TextChanged,InsertLeave <buffer> unlet! b:RstFoldCache
  augroup END
endif

The issues stems from here

" syntax/rst.vim (line 230)
exe 'syn include @rst'.s:filetype.' syntax/'.s:filetype.'.vim'

since

" syntax/javascript.vim (line 82)
setlocal foldmethod=syntax

This is a recurring issue and the adhoc fix could be similar; though there should be a more generic solution for stopping embedded syntax filetypes overriding filetype local settings. Implementing such an adhoc fix would then be up @marshallward (respectively @TysonAndre for PHP)

@Konfekt
Copy link
Contributor

Konfekt commented Nov 19, 2024

though there should be a more generic solution for stopping embedded syntax filetypes overriding filetype local settings.

Maybe simply moving them to ftplugin/*.vim, possibly to an autocmd later checking if syntax highlighting of the corresponding filetype is enabled

@chrisbra
Copy link
Member

I think the javascript syntax file should not set foldmethod foldtext. That belongs into a filetype plugin behind a configuration variable.

@clason
Copy link
Contributor

clason commented Nov 19, 2024

Same for php. I just did a grep for setlocal in runtime/syntax with appalling results. I think a general cleanup may be called for. Excluding (the many hits for) isk[eyword], I get

java.vim:  setlocal foldtext=JavaSyntaxFoldTextExpr()
java.vim:  setlocal foldtext=s:JavaSyntaxFoldTextExpr()
javascript.vim:    setlocal foldmethod=syntax
javascript.vim:    setlocal foldtext=getline(v:foldstart)
fortran.vim:    setlocal foldmethod=syntax
nroff.vim:setlocal paragraphs+=XP
nroff.vim:      setlocal sections=EQTSPS[\ G1GS
nasm.vim:setlocal comments=:;
obse.vim:setlocal foldmethod=syntax
hitest.vim:setlocal autoindent noexpandtab formatoptions=t shiftwidth=18 noswapfile tabstop=18
dirpager.vim:setlocal nowrap
sicad.vim:setlocal expandtab
zsh.vim:    setlocal foldmethod=syntax
ruby.vim:  setlocal foldmethod=syntax
plsql.vim:    setlocal foldmethod=syntax
fasm.vim:setlocal isident=a-z,A-Z,48-57,.,_
dylan.vim:setlocal lisp
eiffel.vim:"    setlocal foldmethod=indent
qml.vim:  setlocal foldmethod=syntax
qml.vim:  setlocal foldtext=getline(v:foldstart)
r.vim:  setlocal foldmethod=syntax
baan.vim:setlocal foldmethod=syntax
occam.vim:setlocal shiftwidth=2
occam.vim:setlocal softtabstop=2
occam.vim:setlocal expandtab
php.vim:  setlocal foldmethod=syntax
php.vim:    setlocal foldmethod=syntax
pf.vim:setlocal foldmethod=syntax
upstreamrpt.vim:setlocal foldmethod=syntax
gift.vim:setlocal conceallevel=1
lpc.vim:setlocal cindent
lpc.vim:setlocal fo-=t fo+=croql
lpc.vim:setlocal comments=sO:*\ -,mO:*\ \ ,exO:*/,s1:/*,mb:*,ex:*/,://
clojure.vim:    setlocal foldmethod=syntax
qf.vim:  setlocal conceallevel=3 concealcursor=nc

@Konfekt
Copy link
Contributor

Konfekt commented Nov 19, 2024

Why should &iskeyword be set in syntax/*.vim? It should at least be reset, shouldn't it?
Don't forget set

redif.vim|967 col 43| set foldmethod=syntax
racket.vim|18 col 3| set iskeyword=33,35-39,42-58,60-90,94,95,97-122,126,_
progress.vim|25 col 1| set expandtab
natural.vim|147 col 3| set foldignore=*
lisp.vim|28 col 2| set ignorecase
cdl.vim|51 col 1| set foldmethod=expr
cdl.vim|52 col 1| set foldexpr=(getline(v:lnum+1)=~'{'\|\|getline(v:lnum)=~'//\\s\\*\\{5}.*table')?'>1':1
cdl.vim|54 col 1| set foldmethod=manual
2html.vim|28 col 1| set eventignore+=FileType
2html.vim|725 col 1| set notitle noicon
2html.vim|727 col 1| set nomore
2html.vim|728 col 1| set report=1000000
2html.vim|818 col 1| set paste
2html.vim|820 col 1| set magic
2html.vim|1155 col 5| set laststatus=2
dockerfile.vim|38 col 1| set commentstring=#\ %s

@clason
Copy link
Contributor

clason commented Nov 19, 2024

I don't think it should; the list would just have been too large to paste here.

@chrisbra
Copy link
Member

Why should &iskeyword be set in syntax/*.vim?

That's probably mostly old syntax file from before :syn iskeyword was available.

In general however those changes should come via their respective maintainer (at least give them a chance to see it) before we make changes here.

@Konfekt
Copy link
Contributor

Konfekt commented Nov 19, 2024

For reference, excluding &iskeyword, that would include author(s), maintainer(s) or updater(s) of

java.vim javascript.vim fortran.vim nroff.vim nasm.vim obse.vim hitest.vim dirpager.vim sicad.vim zsh.vim ruby.vim plsql.vim fasm.vim dylan.vim eiffel.vim qml.vim r.vim baan.vim occam.vim php.vim pf.vim upstreamrpt.vim gift.vim lpc.vim clojure.vim qf.vim redif.vim racket.vim progress.vim natural.vim lisp.vim cdl.vim 2html.vim dockerfile.vim 

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

5 participants