wrong example at `:help undo_ftplugin`

[lacygoill]

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 to 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 the string '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

[lacygoill]

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?

[lacygoill]

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.

[lacygoill]

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

[lacygoill]

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>")

[brammool]

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.

[lacygoill]

Understood, closing then.