TimUntersberger / nog

A tiling window manager for Windows
MIT License
697 stars 20 forks source link

Toggle functions not working #276

Closed keepitsane closed 3 years ago

keepitsane commented 3 years ago

I am trying to create a toggle function for the taskbar, but can't seem to get it to work. Even using the example listed in the lua migration guide doesn't seem to work for me.

Here is the relevant parts of my init.lua

nog.config.display_app_bar = true
nog.config.remove_task_bar = true

nog.nbind("alt+control+i", function ()
    nog.config.remove_task_bar = not nog.config.remove_task_bar
end)

nog.nbind("alt+t", function ()
    nog.config.display_app_bar = not nog.config.display_app_bar
end)

In the log file it seems Nog is receiving the keybinding, but still nothing happens.

[2021-06-21 12:50:38.525798 -04:00] DEBUG [twm\src\main.rs:1085] Received keybinding Keybinding(ALT+CONTROL+I, 72, Work, 3073, None)
[2021-06-21 12:50:39.352874 -04:00] DEBUG [twm\src\main.rs:1085] Received keybinding Keybinding(ALT+T, 73, Normal, 1084, None)
TimUntersberger commented 3 years ago

This might be a problem with the hot reloading.

keepitsane commented 3 years ago

I did try to completely restart Nog a few times and still couldn't get it to work.

But I will say the hot reloading does seem to also be less stable. I can't really pin point what is wrong, but it feels like it is not always working especially compared to the nogscript hot reloading. Maybe it could be useful to display a popup notification to indicate that the hot reload has successfully taken place?

keepitsane commented 3 years ago

Some additional details. When I was in work mode and try to toggle the task bar on, nothing happens. When I exited work mode I noticed that my task bar was missing. I entered work mode again and toggle the task bar, and after exiting work mode the task bar was back. This same thing happens with the app bar toggle. It seems that it is not reflecting the toggles changes in work mode.

ramirezmike commented 3 years ago

I noticed this too and started looking at it a little bit

I made a branch here that's like the initial work.

I noticed that..

A) the toggle/increment/decrement functions weren't hooked up B) the hot reloading function is never called

In that branch I linked above I added the toggle function in and replaced the ReloadConfig event handler to instead call the hot reload just to test out that it works. I was making it toggle my app_bar like this

nog.nbind("alt+i", function() 
    nog.toggle_config("display_app_bar")
end)

It works but only if I'm on a workspace that is actively managing windows. I'm not sure why that is yet...

@TimUntersberger I added some comments on that commit I linked above

TimUntersberger commented 3 years ago

@ramirezmike the thing is the config object is actually a proxy that should automatically tell us when the config has been changed and then call the hot reload function. Having an extra toggle_config shouldn't be needed.

TimUntersberger commented 3 years ago

haha. hot reloading is working, but when we switched to lua I forgot to add the edge cases. I'll open a PR, so we can track this easily.

ramirezmike commented 3 years ago

@ramirezmike the thing is the config object is actually a proxy that should automatically tell us when the config has been changed and then call the hot reload function. Having an extra toggle_config shouldn't be needed.

Oh, right, so, when I first looked at it I thought "the config object behaves like it's read only".

Like, after looking at the documentation I thought you could make a lua function that changes a value using the config table but when that didn't work I started looking at hooking up those functions. Are you saying that config table shouldn't behave like it's read-only?

TimUntersberger commented 3 years ago

It isn't readonly actually haha. The thing is the proxy is working, but we currently don't handle transitions.

Basically you are actually changing the config value, but we dont remove/add the appbar for example.

277 will fix this.

ramirezmike commented 3 years ago

cool cool. I'll delete the branch I made.