[vim/vim] runtime:compilers: ensure compiler! sets global options (PR #14336)
Enno
Previously some options were only set locally by
&l:makeprg/errorformat
This suffices for :compiler (without a trailing bang) but falls short for :compiler! that sets &g:makeprg/errorformat as well
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/14336
Commit Summary
- 6eb174f runtime:compilers: ensure compiler! sets global options
File Changes
(6 files)
- M runtime/compiler/bdf.vim (9)
- M runtime/compiler/context.vim (3)
- M runtime/compiler/mcs.vim (9)
- M runtime/compiler/modelsim_vcom.vim (5)
- M runtime/compiler/powershell.vim (4)
- M runtime/compiler/tex.vim (3)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
Just noting that at upstream https://github.com/PProvost/vim-ps1/blob/master/compiler/powershell.vim this is already corrected but not here
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
@dkearns requested changes on this pull request.
Thanks @Konfekt.
Could you take this opportunity to change the "Latest Revision: " headers to "Last Change: " and also add a "by Enno Nagel" after the date or, if you're shy, "by The Vim Project"?
In runtime/compiler/context.vim:
> @@ -20,6 +20,7 @@ g:current_compiler = 'context'
if get(b:, 'context_ignore_makefile', get(g:, 'context_ignore_makefile', 0)) ||
(!filereadable('Makefile') && !filereadable('makefile'))
&l:makeprg = join(context.ConTeXtCmd(shellescape(expand('%:p:t'))), ' ')
+ execute 'CompilerSet makeprg='. escape(&l:makeprg, ' ')
This file is Vim9-script so it has stricter syntax. Can you please add whitespace before the concat operator and use ...
I think it would be better just to use a local variable makeprg for all of these rather than set the local option value just to read it again and call CompilerSet.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt pushed 1 commit.
- a19c7ef apply kind suggestions by @dkearns
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt commented on this pull request.
In runtime/compiler/context.vim:
> @@ -20,6 +20,7 @@ g:current_compiler = 'context'
if get(b:, 'context_ignore_makefile', get(g:, 'context_ignore_makefile', 0)) ||
(!filereadable('Makefile') && !filereadable('makefile'))
&l:makeprg = join(context.ConTeXtCmd(shellescape(expand('%:p:t'))), ' ')
+ execute 'CompilerSet makeprg='. escape(&l:makeprg, ' ')
Thank you @dkearns, I applied your suggestions.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
Since upstream https://github.com/PProvost/vim-ps1/blob/master/compiler/powershell.vim might also have other improvements, it could still be preferred to use their latest version instead.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
Just noting for reference that the maintainers of tex.vim and modelsim_vcom.vim could not be reached by email
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Lifepillar
@lifepillar commented on this pull request.
In runtime/compiler/context.vim:
> -# Latest Revision: 2023 Dec 26 +# Last Change: 2024 Mar 29 by Enno Nagel
I'd suggest this instead:
Contributors: Enno Nagel
Last Change: 2024 Mar 29
So that the contributor's name won't be lost at the next update.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt commented on this pull request.
In runtime/compiler/context.vim:
> -# Latest Revision: 2023 Dec 26 +# Last Change: 2024 Mar 29 by Enno Nagel
Well, Peter Collingbourne would already be victim to this pull request
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt commented on this pull request.
In runtime/compiler/context.vim:
> -# Latest Revision: 2023 Dec 26 +# Last Change: 2024 Mar 29 by Enno Nagel
.. or, less drama, rather the written memory of his contribution in the compiler file
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lifepillar
@lifepillar commented on this pull request.
In runtime/compiler/context.vim:
> -# Latest Revision: 2023 Dec 26 +# Last Change: 2024 Mar 29 by Enno Nagel
I remember Bram wanting former maintainers to remain in the files. I think it's fair to leave all the names that are already there.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt pushed 1 commit.
- 54ec237 apply suggestion by @lifepillar preserving memory of contributors
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt commented on this pull request.
In runtime/compiler/context.vim:
> -# Latest Revision: 2023 Dec 26 +# Last Change: 2024 Mar 29 by Enno Nagel
Yes, seems so sensible I applied your suggestions in the latest iteration
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dkearns
Thanks @Konfekt. Could you also apply the following patch? I shouldn't have mentioned the variable type change under just one of the file diffs.
I also updated all the string concat operators to use the new .. version and I think we should try and use Vim9-script whitespace conventions in legacy script if the existing file style doesn't dictate a deviation in the interests of consistency.
The "Last Change: ... by HelpfulUser42 " was used by Bram, IIRC, to indicate a change unapproved by the maintainer. These updates often came in via email as well so they were a bit harder to track than they are now. "Contributers:" is, of course, fine and since @Konfekt went the extra mile and notified the maintainers it's a moot point.
diff --git a/runtime/compiler/powershell.vim b/runtime/compiler/powershell.vim index 5625f85a2..445a2f6d9 100644 --- a/runtime/compiler/powershell.vim +++ b/runtime/compiler/powershell.vim @@ -38,7 +38,7 @@ let g:ps1_efm_show_error_categories = get(g:, 'ps1_efm_show_error_categories', 0 " Use absolute path because powershell requires explicit relative paths " (./file.ps1 is okay, but # expands to file.ps1) -let &l:makeprg = g:ps1_makeprg_cmd .' %:p:S' +let makeprg = g:ps1_makeprg_cmd .. ' %:p:S' " Parse file, line, char from callstacks: " Write-Ouput : The term 'Write-Ouput' is not recognized as the name of a @@ -51,7 +51,7 @@ let &l:makeprg = g:ps1_makeprg_cmd .' %:p:S' " + CategoryInfo : ObjectNotFound: (Write-Ouput:String) [], CommandNotFoundException " + FullyQualifiedErrorId : CommandNotFoundException -execute 'CompilerSet makeprg='. escape(&l:makeprg, ' ') +execute 'CompilerSet makeprg=' .. escape(makeprg, ' ') " Showing error in context with underlining. CompilerSet errorformat=%+G+%m diff --git a/runtime/compiler/tex.vim b/runtime/compiler/tex.vim index 7d624133c..45ec61a9f 100644 --- a/runtime/compiler/tex.vim +++ b/runtime/compiler/tex.vim @@ -28,8 +28,8 @@ if exists('b:tex_ignore_makefile') || exists('g:tex_ignore_makefile') || else let current_compiler = "latex" endif - let &l:makeprg=current_compiler.' -interaction=nonstopmode' - execute 'CompilerSet makeprg='. escape(&l:makeprg, ' ') + let makeprg = current_compiler .. ' -interaction=nonstopmode' + execute 'CompilerSet makeprg=' .. escape(makeprg, ' ') else let current_compiler = 'make' endif
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt pushed 1 commit.
- cadd05a apply suggestions by @dkearns
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
@dkearns requested changes on this pull request.
> @@ -11,9 +12,12 @@ let current_compiler = "bdf"
let s:cpo_save = &cpo
set cpo-=C
-setlocal makeprg=bdftopcf\ $*
+if exists(":CompilerSet") != 2 # Older Vim always used :setlocal
The comment char needs to be " not #.
In runtime/compiler/context.vim:
> @@ -3,7 +3,8 @@ vim9script # Language: ConTeXt typesetting engine # Maintainer: Nicola Vitacolonna <nvitac...@gmail.com> # Former Maintainers: Nikolai Weibull <n...@bitwi.se> -# Latest Revision: 2023 Dec 26 +" Contributors: Enno Nagel
Comment char needs to be #.
> @@ -12,7 +12,12 @@ let current_compiler = "mcs"
let s:cpo_save = &cpo
set cpo-=C
-setlocal errorformat=
+if exists(":CompilerSet") != 2 # Older Vim always used :setlocal
Comment char needs to be "
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt pushed 1 commit.
- c040841 fix comment formats as kindly pointed out by @dkearns
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
@Konfekt commented on this pull request.
> @@ -12,7 +12,12 @@ let current_compiler = "mcs"
let s:cpo_save = &cpo
set cpo-=C
-setlocal errorformat=
+if exists(":CompilerSet") != 2 # Older Vim always used :setlocal
Fixed. Thank you very much, and please pardon my hastiness.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dkearns
@dkearns approved this pull request.
Thanks @Konfekt
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
thanks, I'll merge it. It looks like the :CompilerSet command was introduced with at least Vim 7, which was released around 2004. So not sure we need to do the if exists(":CompilerSet") != 2 # Older Vim always used :setlocal - dance anymore.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Merged #14336 into master.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.