Skip to content

Luacheck suggestions#11

Open
bjornbm wants to merge 1 commit intophilanc:masterfrom
bjornbm:luacheck
Open

Luacheck suggestions#11
bjornbm wants to merge 1 commit intophilanc:masterfrom
bjornbm:luacheck

Conversation

@bjornbm
Copy link
Contributor

@bjornbm bjornbm commented Apr 28, 2023

This first commit avoid shadowing variables, ref #8 (comment)

Luacheck reports a number of other things. Removing trailing whitespace is one, but I agree with you that it is not important (and the suggestion can be disabled).

Complaining about globals #10 is another one.

Another one, which I think is quite meaningful as it can catch a mismatch between intent and behavior, is reporting unused variables or arguments. Some examples:

ple.lua:561:9: unused variable ci
ple.lua:723:6: unused loop variable i
ple.lua:768:23: unused argument b 

For the first two, the idea would be to replace the binding to ci or i with bindings to _ which I understand is the convention to say explicitly that you are not interested in the value (as opposed to not using it due to a typo or other mistake in later code). For example, line 561 would become:

		local _, cj = b:getcur()

line 723:

	for _, bx in ipairs(editor.buflist) do

For line 768 (and several similar ones), I understand it the function will likely be called with the current buffer b (from a key binding), but the b is not used. I could understand why you might want to keep the argument to show the expected way it will be called.

function e.nextbuffer(b)
	-- switch to next buffer
	local bln = #editor.buflist
	editor.bufindex = editor.bufindex % bln + 1
	editor.buf = editor.buflist[editor.bufindex]
	editor.fullredisplay()
end--nextbuffer

In any case, if you are interested I can push more commits for things like the unused variables.

Note that I haven't run luacheck over pleterm.lua or buffer.lua. I thought it best to limit to ple.lua until I know what may interest you in terms of pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant