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

[WIP] cfg_xrandr.lua: use local scope where appropriate (fixes #212) #218

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wilhelmy
Copy link
Collaborator

@wilhelmy wilhelmy commented Nov 6, 2019

No description provided.

@wilhelmy
Copy link
Collaborator Author

wilhelmy commented Nov 6, 2019

I'll finalize this before 4.0 but right now there's issues with the tests using local functions from the main script.

@wilhelmy
Copy link
Collaborator Author

wilhelmy commented Nov 7, 2019

FWIW this removes about 20 globals but apparently I'll have to debug it now ;(

@raboof
Copy link
Owner

raboof commented Nov 7, 2019

Looks like test_xrandr.lua wants singleton and is_scratchpad to be available.

Though it'd also be a good fix if we could somehow make those functions available to test_xrandr.lua without making them global.

Testing test_xrandr.lua

/usr/bin/lua5.2: test_xrandr.lua:45: attempt to call global 'singleton' (a nil value)

stack traceback:

	test_xrandr.lua:45: in main chunk

	[C]: in ?

Testing test_xrandr_remove_screen.lua

/usr/bin/lua5.2: cfg_xrandr.lua:8: attempt to call global 'is_scratchpad' (a nil value)

stack traceback:

	cfg_xrandr.lua:8: in function 'getInitialOutputs'

	cfg_xrandr.lua:204: in function 'fn'

	cfg_xrandr.lua:51: in function 'for_all_workspaces_do'

	cfg_xrandr.lua:203: in function 'rearrangeworkspaces'

	test_xrandr_remove_screen.lua:109: in main chunk

	[C]: in ?

Makefile:52: recipe for target 'test' failed

@raboof
Copy link
Owner

raboof commented Nov 7, 2019

(this was near the bottom of the travis console output)

@knixeur
Copy link
Collaborator

knixeur commented Nov 7, 2019

You could export the needed methods in one table to just add one global (iirc is the "best practice") adding at the end:

cfg_randr_export = {
  singleton = singleton,
  is_scratchpad = is_scratchpad,
}

Then use cfg_randr_export.singleton/is_scratchpad where necessary.

Edit: I had to to this to test statusd_inetaddr2 module.

@wilhelmy
Copy link
Collaborator Author

wilhelmy commented Nov 7, 2019

Other than the travis build failure it also seems to be broken for me when I attach/detach monitors. If I restart notion with the old version of the script, all is fine. I think this will require some digging. Marking as WIP.

@wilhelmy wilhelmy changed the title cfg_xrandr.lua: use local scope where appropriate (fixes #212) [WIP] cfg_xrandr.lua: use local scope where appropriate (fixes #212) Nov 7, 2019
@knixeur
Copy link
Collaborator

knixeur commented Nov 7, 2019

firstKey and firstValue are also called from test_xrandr

edit: candidate_screens_for_output

@wilhelmy
Copy link
Collaborator Author

wilhelmy commented Nov 9, 2019

If you're eager to release 4.0, I don't want to stall it. I'll try to make time to fix this but I can't make promises right now. Sorry!

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.

3 participants