~martanne/devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

[PATCH] Fail silently when syntax has no lexer

Michiel van den Heuvel <michielvdnheuvel@gmail.com>
Details
Message ID
<171675157765.21032.3157857469040081524@surya.lan>
DKIM signature
pass
Download raw message
Patch: +9 -12
This'll patch vis.lexers.load to return nil when the lexer could not
be found. Previously it would've errored out -- which the load in
lexer.lua still will -- as this is used in lexers themselves.

Another possibility is to only patch set_syntax in vis.lua and the
WIN_HIGHLIGHT handler in vis-std.lua, but as most references to
vis.lexers.load already handle a nil return, this seems better.

---
 lua/plugins/filetype.lua         | 10 +---------
 lua/plugins/textobject-lexer.lua |  6 +++++-
 lua/vis.lua                      |  5 +++--
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/lua/plugins/filetype.lua b/lua/plugins/filetype.lua
index 553e79d..80fe37e 100644
--- a/lua/plugins/filetype.lua
+++ b/lua/plugins/filetype.lua
@@ -506,15 +506,7 @@ vis.events.subscribe(vis.events.WIN_OPEN, function(win)
		for _, cmd in pairs(filetype.cmd or {}) do
			vis:command(cmd)
		end
		if not vis.lexers.property then return end
		local path = vis.lexers.property['scintillua.lexers']:gsub(';', '/?.lua;')
		local lexname = filetype.alt_name or syntax
		local lexpath = package.searchpath(lexname, path)
		if lexpath ~= nil then
			win:set_syntax(lexname)
		else
			win:set_syntax(nil)
		end
		win:set_syntax(syntax)
	end

	local path = win.file.name -- filepath
diff --git a/lua/plugins/textobject-lexer.lua b/lua/plugins/textobject-lexer.lua
index 2f9d757..eba65e2 100644
--- a/lua/plugins/textobject-lexer.lua
+++ b/lua/plugins/textobject-lexer.lua
@@ -8,13 +8,17 @@ vis:textobject_new("ii", function(win, pos)
		return nil
	end

	local lexer = vis.lexers.load(win.syntax, nil, true)
	if not lexer then
		return nil
	end

	local before, after = pos - MAX_CONTEXT, pos + MAX_CONTEXT
	if before < 0 then
		before = 0
	end
	-- TODO make sure we start at a line boundary?

	local lexer = vis.lexers.load(win.syntax, nil, true)
	local data = win.file:content(before, after - before)
	local tokens = lexer:lex(data)
	local cur = before
diff --git a/lua/vis.lua b/lua/vis.lua
index 5473f17..d06bbaf 100644
--- a/lua/vis.lua
+++ b/lua/vis.lua
@@ -131,7 +131,8 @@ else
	local load_lexer = vis.lexers.load
	vis.lexers.load = function (name, alt_name, cache)
		if cache and lexers[alt_name or name] then return lexers[alt_name or name] end
		local lexer = load_lexer(name, alt_name)
		local status, lexer = pcall(load_lexer, name, alt_name)
		if not status then return nil end
		if cache then lexers[alt_name or name] = lexer end
		return lexer
	end
@@ -276,6 +277,7 @@ vis.types.window.set_syntax = function(win, syntax)
		win.syntax = nil
		return true
	end
	win.syntax = syntax

	if not lexers.load then return false end
	local lexer = lexers.load(syntax)
@@ -297,7 +299,6 @@ vis.types.window.set_syntax = function(win, syntax)
		if style ~= nil then win:style_define(id, style) end
	end

	win.syntax = syntax
	return true
end

-- 
2.39.3 (Apple Git-146)
Details
Message ID
<2Y0DJT9QNU7QB.1Y7UX79A46900@rnpnr.xyz>
In-Reply-To
<171675157765.21032.3157857469040081524@surya.lan> (view parent)
DKIM signature
permerror
Download raw message
Michiel van den Heuvel <michielvdnheuvel@gmail.com> wrote:
> This'll patch vis.lexers.load to return nil when the lexer could not
> be found. Previously it would've errored out -- which the load in
> lexer.lua still will -- as this is used in lexers themselves.
> 
> Another possibility is to only patch set_syntax in vis.lua and the
> WIN_HIGHLIGHT handler in vis-std.lua, but as most references to
> vis.lexers.load already handle a nil return, this seems better.
> 
> ---
>  lua/plugins/filetype.lua         | 10 +---------
>  lua/plugins/textobject-lexer.lua |  6 +++++-
>  lua/vis.lua                      |  5 +++--
>  3 files changed, 9 insertions(+), 12 deletions(-)

Hi Michiel,

Can you please give an example filetype where this is an issue? Or
at least a better description of the issue it attempts to solve?

> 
> diff --git a/lua/plugins/filetype.lua b/lua/plugins/filetype.lua
> index 553e79d..80fe37e 100644
> --- a/lua/plugins/filetype.lua
> +++ b/lua/plugins/filetype.lua
> @@ -506,15 +506,7 @@ vis.events.subscribe(vis.events.WIN_OPEN, function(win)
>  		for _, cmd in pairs(filetype.cmd or {}) do
>  			vis:command(cmd)
>  		end
> -		if not vis.lexers.property then return end
> -		local path = vis.lexers.property['scintillua.lexers']:gsub(';', '/?.lua;')
> -		local lexname = filetype.alt_name or syntax
> -		local lexpath = package.searchpath(lexname, path)
> -		if lexpath ~= nil then
> -			win:set_syntax(lexname)
> -		else
> -			win:set_syntax(nil)
> -		end
> +		win:set_syntax(syntax)

This breaks the alt_name use case which can be used for applying
configuration to certain file types but using the lexer from
another file type. This is currently used for example with the
'git-commit' filetype to set the colour column but to use the
syntax highlighting from the 'diff' lexer.

>  	end
>  
>  	local path = win.file.name -- filepath
> diff --git a/lua/plugins/textobject-lexer.lua b/lua/plugins/textobject-lexer.lua
> index 2f9d757..eba65e2 100644
> --- a/lua/plugins/textobject-lexer.lua
> +++ b/lua/plugins/textobject-lexer.lua
> @@ -8,13 +8,17 @@ vis:textobject_new("ii", function(win, pos)
>  		return nil
>  	end
>  
> +	local lexer = vis.lexers.load(win.syntax, nil, true)
> +	if not lexer then
> +		return nil
> +	end
> +
>  	local before, after = pos - MAX_CONTEXT, pos + MAX_CONTEXT
>  	if before < 0 then
>  		before = 0
>  	end
>  	-- TODO make sure we start at a line boundary?
>  
> -	local lexer = vis.lexers.load(win.syntax, nil, true)
>  	local data = win.file:content(before, after - before)
>  	local tokens = lexer:lex(data)
>  	local cur = before
> diff --git a/lua/vis.lua b/lua/vis.lua
> index 5473f17..d06bbaf 100644
> --- a/lua/vis.lua
> +++ b/lua/vis.lua
> @@ -131,7 +131,8 @@ else
>  	local load_lexer = vis.lexers.load
>  	vis.lexers.load = function (name, alt_name, cache)
>  		if cache and lexers[alt_name or name] then return lexers[alt_name or name] end
> -		local lexer = load_lexer(name, alt_name)
> +		local status, lexer = pcall(load_lexer, name, alt_name)
> +		if not status then return nil end
>  		if cache then lexers[alt_name or name] = lexer end
>  		return lexer
>  	end
> @@ -276,6 +277,7 @@ vis.types.window.set_syntax = function(win, syntax)
>  		win.syntax = nil
>  		return true
>  	end
> +	win.syntax = syntax
>  
>  	if not lexers.load then return false end
>  	local lexer = lexers.load(syntax)
> @@ -297,7 +299,6 @@ vis.types.window.set_syntax = function(win, syntax)
>  		if style ~= nil then win:style_define(id, style) end
>  	end
>  
> -	win.syntax = syntax
>  	return true
>  end
>  


-- 
https://rnpnr.xyz/
GPG Fingerprint: B8F0 CF4C B6E9 415C 1B27 A8C4 C8D2 F782 86DF 2DC5
Michiel van den Heuvel <michielvdnheuvel@gmail.com>
Details
Message ID
<171689329420.39282.6707580554829377040@surya>
In-Reply-To
<2Y0DJT9QNU7QB.1Y7UX79A46900@rnpnr.xyz> (view parent)
DKIM signature
pass
Download raw message
> Can you please give an example filetype where this is an issue? Or
> at least a better description of the issue it attempts to solve?

My bad, I hoped the References header would give some more context.
Still new to mailing lists. This is an implementation of what I
proposed in my mail dated May 8th in the #1190 thread, quoted here
for your convenience:

> I'd like to not even have a warning on the status line. There are a
> few plugins, like vis-lspc and vis-format that rely on the syntax
> field in win to run the correct language server or formatter
> respectively. vis-filetype-settings will also benefit, and there
> might be more.
>
> Personally I think having the detected filetype available without
> any warnings even when there is no lexer available benefits these
> plugins by allowing them to use the existing filetype detection.
> For example, I have a stub hcl.lua lexer for Terraform files. This
> enables me to run the required tools automatically. I'd like to be
> able to share this configuration (and others) as a default in
> vis-format and vis-lspc without having to write a lexer.

> > diff --git a/lua/plugins/filetype.lua b/lua/plugins/filetype.lua
> > index 553e79d..80fe37e 100644
> > --- a/lua/plugins/filetype.lua
> > +++ b/lua/plugins/filetype.lua
> > @@ -506,15 +506,7 @@ vis.events.subscribe(vis.events.WIN_OPEN, function(win)
> >               for _, cmd in pairs(filetype.cmd or {}) do
> >                       vis:command(cmd)
> >               end
> > -             if not vis.lexers.property then return end
> > -             local path = vis.lexers.property['scintillua.lexers']:gsub(';', '/?.lua;')
> > -             local lexname = filetype.alt_name or syntax
> > -             local lexpath = package.searchpath(lexname, path)
> > -             if lexpath ~= nil then
> > -                     win:set_syntax(lexname)
> > -             else
> > -                     win:set_syntax(nil)
> > -             end
> > +             win:set_syntax(syntax)
>
> This breaks the alt_name use case which can be used for applying
> configuration to certain file types but using the lexer from
> another file type. This is currently used for example with the
> 'git-commit' filetype to set the colour column but to use the
> syntax highlighting from the 'diff' lexer.

My bad, missed that one. My own configuration worked around the bug I
created this way. Oops. Fix is trivial, and in the next mail.
Reply to thread Export thread (mbox)