Skip to content
This repository has been archived by the owner on Jan 11, 2020. It is now read-only.

AwesomeWM: To replace/override type() or not? #466

Closed
psychon opened this issue Jan 4, 2018 · 5 comments
Closed

AwesomeWM: To replace/override type() or not? #466

psychon opened this issue Jan 4, 2018 · 5 comments

Comments

@psychon
Copy link
Contributor

psychon commented Jan 4, 2018

AwesomeWM replaces Lua's type function so that e.g. type(screen[1]) will return "screen" (see luaAe_fixups and luaAe_type in luaa.c). In way-cooler, this will result in "table" instead.

This breaks e.g. the compatibility stuff in awful.taglist.new: In older versions of awesome, this function got six arguments which all were basically optional. These days it gets a table instead in which it looks up its arguments. Based on type(first_argument) == "screen" it provides backward-compatibility with the old arguments list. Since the rc.lua that comes with way-cooler uses an old-style call... well, I got a cryptic error about table index is nil in a line that used a screen as a table index because the code tried to use argument.screen as the screen, where argument was already supposed to be a screen.

@Timidger
Copy link
Member

Timidger commented Jan 4, 2018

Hmm, that is a problem.

I'm hoping that we can eventually have the same structure as old awesome if the points I made in this old issue are addressed. Since there's little response there (unsurprising with the holiday season, and it is an old "solved" issue) I'm thinking I'm might try to tackle it next week and send a pr to them. The main thing is trying to achieve it safely, since it won't be merged if it exposes a safety hole in the library.

If that doesn't work out, there probably is a way we can get the custom type checking working with the table. I'd rather fix the underlying structure though, because I think this issue is going to keep coming up for us otherwise.

@Timidger
Copy link
Member

This should be fixed thanks to this change in rlua. We just need to propogate it here by changing how we store the types as uservalues rather than the tables. Then we don't need to override type.

@Timidger Timidger mentioned this issue Jan 31, 2018
54 tasks
@psychon
Copy link
Contributor Author

psychon commented Jan 31, 2018

Then we don't need to override type.

Uhm, yes, you do. As far as I know, Lua's built-in type always identifies userdata as "userdata". That's why awesome explicitly overwrites _G.type with its own function.

@Timidger
Copy link
Member

Whoops, sorry. Read this issue too quickly last night and obviously wasn't awake enough.

Yes we'll still need to override type

@psychon
Copy link
Contributor Author

psychon commented Feb 6, 2018

This should be fixed by 5003763. Sorry, I forgot to refer to this issue in the commit message.

@psychon psychon closed this as completed Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants