Skip to content

Split file into multiple modules #85

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

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

Conversation

phischdev
Copy link

Split the init.lua file into the following structure:

PaperWM.spoon/
├── init.lua # Main entry point
├── modules/ # Module directory
│ ├── config.lua # Configuration and constants
│ ├── window_manager.lua # Window tracking and management
│ ├── layout_engine.lua # Layout algorithms
│ ├── space_manager.lua # Space navigation
│ └── action_manager.lua # User actions
└── external/
├── mission_control.lua # Mission Control integration
└── swipe.lua # Swipe gesture detection

and added comments to the files (automatically)

@phischdev phischdev changed the title initial split Split file into multiple modules May 2, 2025
@phischdev
Copy link
Author

Hi @mogenson,
I wanted to add some custom functionality to paper wm but found myself unable due to the steep learning curve I experience with a 2000 lines long file.
So i tried to separate the logic into multiple modules that can be understood on their own.

@mogenson
Copy link
Owner

mogenson commented May 3, 2025

Thank you for the long overdue refactor. Let me give it a try and I'll get back to you.

@mogenson
Copy link
Owner

mogenson commented May 3, 2025

Looks like there's an issue with the cycle window command:

2025-05-03 12:07:22: 12:07:22 ERROR:   LuaSkin: hs.hotkey callback: ...merspoon/Spoons/PaperWM.spoon/modules/action_manager.lua:403: attempt to index a nil value (field 'utils')
stack traceback:
	...merspoon/Spoons/PaperWM.spoon/modules/action_manager.lua:403: in function 'modules.action_manager.cycleWindowSize'
	(...tail calls...)

@mogenson
Copy link
Owner

mogenson commented May 3, 2025

I'm also seeing repeating events that don't stop (eg "windowFocused for ") in the Hammerspoon Console. I'll see if I can figure out what's causing them.

@mogenson
Copy link
Owner

mogenson commented May 3, 2025

Would you mind formatting the code? There's a CI code format lint results here: https://github.com/mogenson/PaperWM.spoon/actions/runs/14803256610/job/41584942320?pr=85

The lua-language-server picks up the format config from the top level .luarc.json file. Let me see if there's a way to auto format source files from a CLI command.

@mogenson
Copy link
Owner

mogenson commented May 3, 2025

Would you mind formatting the code? There's a CI code format lint results here: https://github.com/mogenson/PaperWM.spoon/actions/runs/14803256610/job/41584942320?pr=85

The lua-language-server picks up the format config from the top level .luarc.json file. Let me see if there's a way to auto format source files from a CLI command.

Annoyingly I cannot find a way to invoke the code formatter without starting the language server RPC.

Let me know if you have any issues configuring your editor to use https://github.com/LuaLS/lua-language-server

@phischdev
Copy link
Author

Hi @mogenson,
Thanks for looking into it. I linted the whole code, so I hope that action will pass now.

Besides that I think you're right: Some features seem to be buggy now.
I'm not quite sure what the best way to proceed would be now. Do you think it would make sense to restart refactoring from the beginning, now that the master has changed?

@mogenson
Copy link
Owner

Thanks for fixing the lints. Since this is a big change, let's do it piece by piece and ensure no functionality is broken between changes.

For example, first create and move things into modules/config.lua. Then we'll test and merge that.

There's now conflicts with the main branch so please rebase onto main.

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