search

BlingCorp / bling

Utilities for the awesome window manager
https://blingcorp.github.io/bling/
MIT License
851 stars 51 forks source link

Fix handling of signals in scratchpad when client is nil #148

Closed skbolton closed 2 years ago

skbolton commented 2 years ago

Fixes #146 for me. I am pretty new to the codebase and could be missing the intention of this area of the code.

Nooo37 commented 2 years ago

Thanks for bringing that to our attention and sorry for taking so long to take a look at it! The scratchpad holds the client that it is associated with as a field. You are right that it doesn't make much sense as it is right now. Your commit surely fixes it because it connects/disconnects the function from every client there is which is totally fine because the function that gets connected/disconnected only exists for one scratchpad anyway (as it is "dynamically generated" if you will).

That being said, to nitpick a little on your solution: I do think that it would be conceptually more clear if the functions gets connected/disconnected explicitly from one client and not from all clients with the "hidden implication" that there is only one client to disconnect from anyway.

-self.client.disconnect_signal("manage", inital_apply)
+self.client:disconnect_signal("manage", inital_apply)

@Kasper24 Can you verify my assessment?

skbolton commented 2 years ago

Yeah this file was just recently refactored away from the way I have it here, and I think the intention was what you are saying @Nooo37. Its just down this path self.client is nil so there needs to be a new way to come up with connecting the signal to a single client

Nooo37 commented 2 years ago

Oh you are right, my proposal doesn't make much sense either here