[vim/vim] Using compiled functions in ftplugin breaks let statements in b:undo_ftplugin (Issue #9741)
itchyny
Steps to reproduce
- Execute
set ft=rusttwice. - Get following error.
Error detected while processing FileType Autocommands for "*"..function <SNR>5_LoadFTPlugin:
line 2:
E1126: Cannot use :let in Vim9 script
This is likely a breaking change by 3e79c97 .
The current rust ftplugin contains b:undo_ftplugin including let statements (https://github.com/vim/vim/blob/v8.2.4300/runtime/ftplugin/rust.vim#L151).
The undo ftplugin looks the similar problem too.
For compatibility of executing undo_* variables, maybe we should not migrate these functions to Vim9 script.
Expected behaviour
Shows no error.
Version of Vim
8.2.4300
Environment
macOS 12.2, iTerm2 3.4.12
Logs and stack traces
No response
—
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.
itchyny
The undo ftplugin looks the similar problem too.
Oops, I mean runtime/indent.vim.
—
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.
matveyt
I guess that implies executing b:undo_ftplugin (as well as b:undo_indent) in legacy mode. However the problem is that legacy exe b:undo_ftplugin is still executed in the current mode (i.e. vim9script). Is that another bug?
—
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.
Bram Moolenaar
Using "legacy exe" appears to fix it. What other problem do you see?
—
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.
Bram Moolenaar
—
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.
lacygoill
No, the issue persists. That's because :legacy is a modifier. Its effect stops at the next bar. We need to execute the variable from a legacy function. I'll try to send a patch 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.
lacygoill
Actually, maybe it's a bug. Because here, :legacy doesn't stop at the bar:
:legacy execute 'eval 0 is# 0 | eval 0 is# 0'
—
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.
lacygoill
Ah no, I was right, :legacy does stop at the bar in Vim9 context:
vim9script legacy execute 'if true | eval 0 is# 0 | endif'
E15: Invalid expression: "is# 0 | endif"
E171: Missing :endif
—
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.
lacygoill
So, it's a bug. Because other modifiers don't stop at the bar when they apply to an :executed string:
vim9script silent execute 'echomsg "aaa" | echomsg "bbb"'
no message is logged
—
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.
lacygoill
Actually, it's not even the bar which is problematic. It's the fact that the modifier is not applied at all with an :execute. And it's not specific to :legacy. Same issue with :vim9cmd:
:vim9cmd execute 'eval () => 123'
—
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.
lacygoill
I noticed this issue a long time ago, but I thought it was working as intended. I think that :execute automatically runs the command at the script level. Here, the script is written in Vim9 context. Therefore, the command is executed in Vim9 context. IOW, :execute supersedes :legacy and :vim9cmd. The question is: should it? If it should, then there is no bug, and the solution is simply to execute b:undo_ftplugin from a legacy function.
diff --git a/runtime/ftplugin.vim b/runtime/ftplugin.vim index 2500a7f27..01c092e8f 100644 --- a/runtime/ftplugin.vim +++ b/runtime/ftplugin.vim @@ -22,7 +22,7 @@ endif def LoadFTPlugin() if exists("b:undo_ftplugin") # We assume b:undo_ftplugin is using legacy script syntax - legacy exe b:undo_ftplugin + call s:UndoFtplugin() unlet! b:undo_ftplugin b:did_ftplugin endif @@ -41,3 +41,7 @@ def LoadFTPlugin() endfor endif enddef + +function s:UndoFtplugin() abort + exe b:undo_ftplugin +endfunction
—
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.
matveyt
The question is: should it?
IMO, it should NOT. Consider also that both :execute and execute() do supersede, while :eval and eval() do not. This is really confusing.
—
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.
Bram Moolenaar
> I noticed [this issue](https://github.com/lacygoill/wiki/blob/master/bug/vim.md#-72) a long time ago, but I thought it was working as intended. I think that `:execute` automatically runs the command at the script level. Here, the script is written in Vim9 context. Therefore, the command is executed in Vim9 context. IOW, `:execute` supersedes `:legacy` and `:vim9cmd`. The question is: should it? If it should, then there is no bug, and the solution is simply to execute `b:undo_ftplugin` from a legacy function.
> ```diff
> diff --git a/runtime/ftplugin.vim b/runtime/ftplugin.vim
> index 2500a7f27..01c092e8f 100644
> --- a/runtime/ftplugin.vim
> +++ b/runtime/ftplugin.vim
> @@ -22,7 +22,7 @@ endif
> def LoadFTPlugin()
> if exists("b:undo_ftplugin")
> # We assume b:undo_ftplugin is using legacy script syntax
> - legacy exe b:undo_ftplugin
> + call s:UndoFtplugin()
> unlet! b:undo_ftplugin b:did_ftplugin
> endif
>
> @@ -41,3 +41,7 @@ def LoadFTPlugin()
> endfor
> endif
> enddef
> +
> +function s:UndoFtplugin() abort
> + exe b:undo_ftplugin
> +endfunction
This looks more like a workaround than a solution. When using
"legacy exe" I would think most people expect "legacy" to apply to the
commands that "exe" will execute. It's mostly pointless otherwise.
Well, it could apply to evaluating the argument of ":execute" I suppose.
Would someone read it that way?
Let me make this work for :legacy and :vim9script. There might be other
modifiers that need to be handled like this, please come up with
examples if you can think of some.
--
The History of every major Galactic Civilization tends to pass through
three distinct and recognizable phases, those of Survival, Inquiry and
Sophistication, otherwise known as the How, Why and Where phases.
For instance, the first phase is characterized by the question 'How can
we eat?' the second by the question 'Why do we eat?' and the third by
the question 'Where shall we have lunch?'
-- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
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.
Wez Furlong
I just ran into this on Fedora 35 after installing today's critical updates. The error message is not easy to run down: it doesn't show a filename to give clues on what to look at, so I spent some time trying to figure out why vim9 was mentioned when I had vim8.2 and whether this was something I needed to fix in my dotfiles :-(
Is there scope for making that error messaging clearer?
—
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.
Bram Moolenaar
It is working properly after the latest fixes, right?
It's difficult to give better error messages, since there are so many places involved.
—
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.
Wez Furlong
this system just has the fedora vim packages installed, so my "fix" was to downgrade to an earlier version of the rpm(s). I know that's not super helpful for you, sorry!
—
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.
lacygoill
For this kind of reasons, I've been thinking we should have a builtin package which populates the quickfix list with the last errors logged in :messages. Similar to cfilter-plugin.
I have something like that:
But I still need to improve it in a corner case (the parsing doesn't fully handle an error given for a lambda which failed to compile). Also, not sure about the order of the display (first errors at the top vs at the bottom; in a given stack, last frame at the top vs at the bottom).
—
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.
Josh Stone
I just ran into this on Fedora 35 after installing today's critical updates.
It is working properly after the latest fixes, right?
It's working for me with Fedora's update to vim-8.2.4386-1.fc35.
—
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.