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

Awesome 4.3 patch (lua54 compatibility) #232

Merged

Conversation

sage-etcher
Copy link
Contributor

@sage-etcher sage-etcher commented May 2, 2024

Awesome's -v version flag, was written with Lua53's require in mind, which only returns 1 value. However, in Lua54, the require built-in now returns 2 values. Awesome doesn't handle this causing the version information to display incorrectly.

Rather than checking LUA_VERSION_NUM, the patch handles this at runtime, so no matter the version of Lua used at build time, the runtime information should be correct for the given Lua + LGI softwares.

I tested the patch with Lua versions: 5.4.3, 5.3.5, and 5.3.6, and it works in each.

Example (w/o the patch) running Lua 5.4.3 and LGI-GIT:

$ awesome -v
awesome v4.3 (Too long)
 ? Compiled against Lua 5.4.3 (running with 0.9.2)
 ? D-Bus support: ?
 ? execinfo support: ?
 ? xcb-randr version: 1.6
 ? LGI version: /System/Aliens/LuaRocks/share/lua/5.4/lgi/version.lua

Example (w/ the patch)

$ awesome -v
awesome v4.3 (Too long)
 ? Compiled against Lua 5.4.3 (running with Lua 5.4)
 ? D-Bus support: ?
 ? execinfo support: ?
 ? xcb-randr version: 1.6
 ? LGI version: 0.9.2

The patch isn't essential, Awesome will run fine without it, but it's nice having the version information not be wrong ://

for Issue #128

sage-etcher and others added 2 commits May 2, 2024 19:19
Due to a change in Lua's 'request' built-in, Awesome's version flag now
returns bad data when using Lua5.4. This patch handles that change
without breaking earlier implementations, by only looking at the first
returned value of 'request'.
@Nuc1eoN
Copy link
Member

Nuc1eoN commented May 6, 2024

LGTM and looks useful, ty!

@Nuc1eoN Nuc1eoN merged commit f60ff6a into gobolinux:master May 6, 2024
@sage-etcher
Copy link
Contributor Author

sage-etcher commented May 6, 2024

It's unneeded, however, I realized after the fact that I overcomplicated this a bit. I have a simplified (slightly faster) version of this patch as well; I'd forgot to commit it earlier. Would that be something you'd be interested in?

Instead of using the lua_pop macro and a second call to lua_gettop, it manually sets the top, saving a function call and quite a few computations by using

lua_settop(L, top+1);

I think it's also a bit clearer what is going on, for example in an edge-case where require doesn't push anything. In such scenario, previous implementation would say lua_pop(-1) which would push a value onto the stack to align the data, which works but is unclear. However lua_settop is clearer in that it will always make sure the data is aligned by +1.

@Nuc1eoN
Copy link
Member

Nuc1eoN commented May 6, 2024

Sure why not, feel free to submit it:)

@sage-etcher sage-etcher mentioned this pull request May 6, 2024
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.

2 participants