[vim/vim] wrong example at :help undo_ftplugin (Issue #9645)

28 views
Skip to first unread message

lacygoill

unread,
Jan 27, 2022, 8:45:28 PM1/27/22
to vim/vim, Subscribed

From :help undo_ftplugin:

b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"

        \ .. "| unlet b:match_ignorecase b:match_words b:match_skip"

This gives an error:

vim9script

var dir = '/tmp/.vim'

dir->delete('rf')

dir ..= '/after'

&runtimepath ..= ',' .. dir

dir ..= '/ftplugin'

dir->mkdir('p')

var lines =<< trim END

    b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"

            \ .. "| unlet b:match_ignorecase b:match_words b:match_skip"

END

var ft = 'vim'

lines->writefile(printf('%s/%s.vim', dir, ft))

filetype plugin on

&filetype = ft
E94: No matching buffer for :undo_ftplugin =

That's because :let is necessary in legacy, and here it's missing:

diff --git a/runtime/doc/usr_41.txt b/runtime/doc/usr_41.txt

index 71a4850ed..b8841b153 100644

--- a/runtime/doc/usr_41.txt

+++ b/runtime/doc/usr_41.txt

@@ -2502,7 +2502,7 @@ When the user does ":setfiletype xyz" the effect of the previous filetype

 should be undone.  Set the b:undo_ftplugin variable to the commands that will

 undo the settings in your filetype plugin.  Example: >

 

-	b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"

+	let b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"

                \ .. "| unlet b:match_ignorecase b:match_words b:match_skip"

 

 Using ":setlocal" with "<" after the option name resets the option to its

Second, it does not work in a filetype plugin located in an after/ directory:

vim9script

var dir = '/tmp/.vim'

dir->delete('rf')

dir ..= '/after'

&runtimepath ..= ',' .. dir

dir ..= '/ftplugin'

dir->mkdir('p')

var lines =<< trim END

    setlocal shiftwidth=4

    echomsg 'before sourcing after/ ftplugin: ' .. b:undo_ftplugin

    let b:undo_ftplugin = 'setlocal shiftwidth<'

    echomsg 'after sourcing after/ ftplugin:  ' .. b:undo_ftplugin

END

var ft = 'vim'

lines->writefile(printf('%s/%s.vim', dir, ft))

filetype plugin on

&filetype = ft
before sourcing after/ ftplugin: call VimFtpluginUndo()

after sourcing after/ ftplugin:  setlocal shiftwidth<

Notice that the call VimFtpluginUndo() has been lost. That's because we used an assignment which completely resets the value, and prevents options set in other filetype plugins to be properly undone.

We need to append the string, not overwrite it:

diff --git a/runtime/doc/usr_41.txt b/runtime/doc/usr_41.txt

index 71a4850ed..3731476f9 100644

--- a/runtime/doc/usr_41.txt

+++ b/runtime/doc/usr_41.txt

@@ -2502,7 +2502,7 @@ When the user does ":setfiletype xyz" the effect of the previous filetype

 should be undone.  Set the b:undo_ftplugin variable to the commands that will

 undo the settings in your filetype plugin.  Example: >

 

-	b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"

+	let b:undo_ftplugin ..= "| setlocal fo< com< tw< commentstring<"

                \ .. "| unlet b:match_ignorecase b:match_words b:match_skip"

 

 Using ":setlocal" with "<" after the option name resets the option to its

But it's still not correct. What if b:undo_ftplugin does not exist?

vim9script

var dir = '/tmp/.vim'

dir->delete('rf')

dir ..= '/after'

&runtimepath ..= ',' .. dir

dir ..= '/ftplugin'

dir->mkdir('p')

var lines =<< trim END

    let b:undo_ftplugin ..= "| setlocal fo< com< tw< commentstring<"

        \ .. "| unlet b:match_ignorecase b:match_words b:match_skip"

END

var ft = 'test'

lines->writefile(printf('%s/%s.vim', dir, ft))

filetype plugin on

&filetype = ft
E121: Undefined variable: b:undo_ftplugin

An error is given. We need get() to suppress it:

diff --git a/runtime/doc/usr_41.txt b/runtime/doc/usr_41.txt

index 71a4850ed..361692b76 100644

--- a/runtime/doc/usr_41.txt

+++ b/runtime/doc/usr_41.txt

@@ -2502,7 +2502,8 @@ When the user does ":setfiletype xyz" the effect of the previous filetype

 should be undone.  Set the b:undo_ftplugin variable to the commands that will

 undo the settings in your filetype plugin.  Example: >

 

-	b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"

+	let b:undo_ftplugin = get(b:, 'undo_ftplugin', '')

+		\ .. "| setlocal fo< com< tw< commentstring<"

                \ .. "| unlet b:match_ignorecase b:match_words b:match_skip"

 

 Using ":setlocal" with "<" after the option name resets the option to its

It's still not correct:

vim9script

var dir = '/tmp/.vim'

dir->delete('rf')

dir ..= '/after'

&runtimepath ..= ',' .. dir

dir ..= '/ftplugin'

dir->mkdir('p')

var lines =<< trim END

    let b:undo_ftplugin = get(b:, 'undo_ftplugin', '')

        \ .. "| setlocal fo< com< tw< commentstring<"

END

var ft = 'test'

lines->writefile(printf('%s/%s.vim', dir, ft))

filetype plugin on

&filetype = ft

&filetype = ''
E749: empty buffer

This error is given because b:undo_ftplugin starts with a bar. And in legacy, at the start of a line, a bar prints the current line. This is an undesirable effect, which can give the previous unexpected error when the buffer is empty. When we call get() and pass it a default value, we can't just write an empty string. We need something which has no side-effect when executed by execute. The simplest no-op I can think of is execute itself:

:execute 'execute'

So instead of this:

let b:undo_ftplugin = get(b:, 'undo_ftplugin', '')

    \ .. "| setlocal fo< com< tw< commentstring<"

We could write this:

let b:undo_ftplugin = get(b:, 'undo_ftplugin', 'execute')

    \ .. "| setlocal fo< com< tw< commentstring<"

Test:

vim9script

var dir = '/tmp/.vim'

dir->delete('rf')

dir ..= '/after'

&runtimepath ..= ',' .. dir

dir ..= '/ftplugin'

dir->mkdir('p')

var lines =<< trim END

    let b:undo_ftplugin = get(b:, 'undo_ftplugin', 'execute')

        \ .. "| setlocal fo< com< tw< commentstring<"

END

var ft = 'test'

lines->writefile(printf('%s/%s.vim', dir, ft))

filetype plugin on

&filetype = ft

&filetype = ''
no error

It's still not correct, because it doesn't handle the case where b:undo_ftplugin does exist, but is empty:

vim9script

var dir = '/tmp/.vim'

dir->delete('rf')

dir ..= '/after'

&runtimepath ..= ',' .. dir

dir ..= '/ftplugin'

dir->mkdir('p')

var lines =<< trim END

    let b:undo_ftplugin = ''

END

var ft = 'test_aaa'

lines->writefile(printf('%s/%s.vim', dir, ft))

lines =<< trim END

    let b:undo_ftplugin = get(b:, 'undo_ftplugin', 'execute')

        \ .. "| setlocal fo< com< tw< commentstring<"

END

ft = 'test_bbb'

lines->writefile(printf('%s/%s.vim', dir, ft))

filetype plugin on

&filetype = ft->substitute('_.*', '', '')

&filetype = ''
E749: empty buffer

One solution is to use the null coalescing operator to assert that b:undo_ftplugin should not be empty, and fall back on execute otherwise:

let b:undo_ftplugin = (get(b:, 'undo_ftplugin') ?? 'execute')

    \ .. ' | setlocal fo< com< tw< commentstring<'

Test:

vim9script

var dir = '/tmp/.vim'

dir->delete('rf')

dir ..= '/after'

&runtimepath ..= ',' .. dir

dir ..= '/ftplugin'

dir->mkdir('p')

var lines =<< trim END

    let b:undo_ftplugin = ''

END

var ft = 'test_aaa'

lines->writefile(printf('%s/%s.vim', dir, ft))

lines =<< trim END

    let b:undo_ftplugin = (get(b:, 'undo_ftplugin') ?? 'execute')

        \ .. ' | setlocal fo< com< tw< commentstring<'

END

ft = 'test_bbb'

lines->writefile(printf('%s/%s.vim', dir, ft))

filetype plugin on

&filetype = ft->substitute('_.*', '', '')

&filetype = ''

As a suggestion, here is the final patch which – hopefully – handles all corner cases:

diff --git a/runtime/doc/usr_41.txt b/runtime/doc/usr_41.txt

index 71a4850ed..cff84c682 100644

--- a/runtime/doc/usr_41.txt

+++ b/runtime/doc/usr_41.txt

@@ -2502,7 +2502,8 @@ When the user does ":setfiletype xyz" the effect of the previous filetype

 should be undone.  Set the b:undo_ftplugin variable to the commands that will

 undo the settings in your filetype plugin.  Example: >

 

-	b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"

+	let b:undo_ftplugin = (get(b:, 'undo_ftplugin') ?? 'execute')

+		\ .. "| setlocal fo< com< tw< commentstring<"

                \ .. "| unlet b:match_ignorecase b:match_words b:match_skip"

 

 Using ":setlocal" with "<" after the option name resets the option to its

—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/9645@github.com>

lacygoill

unread,
Jan 27, 2022, 9:03:38 PM1/27/22
to vim/vim, Subscribed

The simplest no-op I can think of is execute itself:

Unrelated question: why is it required for :normal to be passed an argument, but not :execute?

:execute
# no error

:normal
E471: Argument required

In Vim9, should we give an error when the argument of :execute is missing?

—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/9645/1023814849@github.com>

lacygoill

unread,
Jan 27, 2022, 9:12:33 PM1/27/22
to vim/vim, Subscribed

Unrelated question: why is it required for :normal to be passed an argument, but not :execute?

Never mind, there are other commands for which we don't check a missing argument. I'll open a separate issue later.

—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/9645/1023818946@github.com>

lacygoill

unread,
Jan 27, 2022, 11:58:55 PM1/27/22
to vim/vim, Subscribed

Back to b:undo_ftplugin, the example could be simplified a lot if $VIMRUNTIME/ftplugin.vim did 2 things:

  • stop deleting b:undo_ftplugin unconditionally; do it only when &filetype is empty
  • reset b:undo_ftplugin to the string 'execute' when &filetype is not empty

How about this patch:

diff --git a/runtime/ftplugin.vim b/runtime/ftplugin.vim
index a434b9372..e0c70c867 100644
--- a/runtime/ftplugin.vim
+++ b/runtime/ftplugin.vim
@@ -12,9 +12,12 @@ augroup filetypeplugin
   au FileType * call s:LoadFTPlugin()
 
   func! s:LoadFTPlugin()
-    if exists("b:undo_ftplugin")
-      exe b:undo_ftplugin
-      unlet! b:undo_ftplugin b:did_ftplugin
+    unlet! b:did_ftplugin
+    if &filetype == ''
+      unlet! b:undo_ftplugin
+    else
+      execute get(b:, 'undo_ftplugin', '')
+      let b:undo_ftplugin = ''
     endif
 
     let s = expand("<amatch>")

If something like this was merged, the example could be re-written like this:

diff --git a/runtime/doc/usr_41.txt b/runtime/doc/usr_41.txt
index 66b267f93..9df3f6f62 100644
--- a/runtime/doc/usr_41.txt
+++ b/runtime/doc/usr_41.txt
@@ -2504,7 +2504,7 @@ When the user does ":setfiletype xyz" the effect of the previous filetype
 should be undone.  Set the b:undo_ftplugin variable to the commands that will
 undo the settings in your filetype plugin.  Example: >
 

-	b:undo_ftplugin = "setlocal fo< com< tw< commentstring<"
+	let b:undo_ftplugin ..= "| setlocal fo< com< tw< commentstring<"
 		\ .. "| unlet b:match_ignorecase b:match_words b:match_skip"
 
 Using ":setlocal" with "<" after the option name resets the option to its



—

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/9645/1023887387@github.com>


lacygoill
unread,
Jan 28, 2022, 12:03:35 AM1/28/22
to vim/vim, Subscribed




How about this patch:




Sorry, wrong patch.  It doesn't initialize b:undo_ftplugin with the string 'execute'.  And it doesn't execute it when &filetype is made empty.


New patch:


diff --git a/runtime/ftplugin.vim b/runtime/ftplugin.vim
index a434b9372..0f2cc6a6f 100644

--- a/runtime/ftplugin.vim
+++ b/runtime/ftplugin.vim
@@ -12,9 +12,12 @@ augroup filetypeplugin
   au FileType * call s:LoadFTPlugin()
 
   func! s:LoadFTPlugin()
-    if exists("b:undo_ftplugin")
-      exe b:undo_ftplugin
-      unlet! b:undo_ftplugin b:did_ftplugin
+    unlet! b:did_ftplugin

+    execute get(b:, 'undo_ftplugin', '')
+    if &filetype == ''
+      unlet! b:undo_ftplugin
+    else

+      let b:undo_ftplugin = 'execute'
     endif
 
     let s = expand("<amatch>")



—

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/9645/1023889272@github.com>


Bram Moolenaar
unread,
Jan 28, 2022, 2:43:04 PM1/28/22
to vim/vim, Subscribed


I'll add the missing "let".


Otherwise, most plugins can just set b:undo_ftplugin.  At least in the example in the user manual we should not care about corner cases.

Having an "after" plugin is unlikely, only a few users would have them.  There it can use exist() to check if the undo variable was already set.  If yes, then append with a bar, otherwise just set it.



—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/9645/1024577801@github.com>


lacygoill
unread,
Jan 28, 2022, 2:53:46 PM1/28/22
to vim/vim, Subscribed


Understood, closing then.



—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/9645/1024586937@github.com>


lacygoill
unread,
Jan 28, 2022, 2:53:48 PM1/28/22
to vim/vim, Subscribed


Closed #9645.



—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issue/9645/issue_event/5973158035@github.com>


ben.k...@gmail.com
unread,
Jan 29, 2022, 11:43:08 AM1/29/22
to vim_dev
On Friday, January 28, 2022 at 2:43:04 PM UTC-5 Bram Moolenaar wrote:





Otherwise, most plugins can just set b:undo_ftplugin.  At least in the example in the user manual we should not care about corner cases.

Having an "after" plugin is unlikely, only a few users would have them.  There it can use exist() to check if the undo variable was already set.  If yes, then append with a bar, otherwise just set it.

Really? I find that this is the most idiomatic way to extend vim's filetype support (and possibly that of any installed plugins). _All_ my custom ftplugin files are in ~/.vim/after. I don't want to prevent other ftplugins; I want to overrule them or extend them on certain matters. So I want to be dead last.


And I do set b:undo_ftplugin. I used to do a complicated dance for edge cases; now I use a custom script which centralizes that, but still has to worry about it because some plugins are bad at setting undo_ftplugin.

Message ID: <vim/vim/issues/9645/1024577801@github.com>


Reply all
Reply to author
Forward
0 new messages