Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luacheck: accessing undefined variable attr, in lua/vis.lua #1215

Open
aimixsaka opened this issue Nov 11, 2024 · 1 comment · May be fixed by #1216
Open

luacheck: accessing undefined variable attr, in lua/vis.lua #1215

aimixsaka opened this issue Nov 11, 2024 · 1 comment · May be fixed by #1216

Comments

@aimixsaka
Copy link

Problem

I'm building vis from source, following https://github.com/martanne/vis/wiki/Developer-Overview#luacheck, got:

...
Checking lua/vis-std.lua                          OK
Checking lua/vis.lua                              1 warning

    lua/vis.lua:291:35: accessing undefined variable attr

Checking lua/visrc.lua                            OK
...

lua/vis.lua:291

...
if style.attr then
        s = string.format("%s,%s", s, attr)
elseif style.fore then
        s = string.format("%s,fore:%s", s, style.fore)
...

Is this a mistake ?

Steps to reproduce

No response

vis version (vis -v)

vis c0d083f +curses +lua +tre +acl

Terminal name/version

No response

$TERM environment variable

No response

@fischerling
Copy link
Contributor

This is definitely an error.
Checking style.attr in line 290 but using the global attr in line 291 is wrong.

However, there is not a single style in the code base of vis defining a style as table.
All style definitions use the string representation directly.
Therefore, this mistake was probably never noticed except a user defines a style as a table.
Which is unlikely because all examples are strings but possible.

Using something like in a custom user theme

lexers.STYLE_CLASS = {fore=colors.yellow, attr='bold'}

does not render the class tokens bold.

@fischerling fischerling linked a pull request Nov 14, 2024 that will close this issue
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 a pull request may close this issue.

2 participants