Make strut cache global #80
Conversation
|
Thanks! Please have a look at https://github.com/liskin/xmonad-contrib/commits/managedocks-global-cache, I made some additional changes, improved your commit message and added another commit. |
|
@liskin I agree with almost all of your improvements except using UPD: And I also think this shouldn't close #76. Yes, now strut problem is solved, but |
|
Well, I did some research and found no modules in xmonad-contrib that use messages solely for the purpose of refreshing the layout without carrying a specific change to its configuration, while there are some that use Regarding #76, I have no idea. Does |
|
I think code is cleaner when something is run only when it needs to be run.
It prevents messages being sent to the layout when it has never been active because it retranslates message to its first sub-layout when it hasn't been run initially while it should retranslate it to the second one. |
Okay, whatever. :-)
Oh. Well, I never claimed that #76 was fixed by this, anyway. Point c) in liskin@fab2c2a is a different issue then, one that is now fixed indeed. |
f1u77y
force-pushed
the
managedocks-global-cache
branch
from
1620ecd to
3b373d6
Compare
Commits d638dc8 and a5e87e3 introduced a per-AvoidStruts-instance strut cache that a) didn't get initialized at startup, b) didn't get reinitialized after layout reset and c) didn't get updates if it wasn't the active layout, for example when layoutHook = avoidStruts tall ||| avoidStruts (mirror tall) a) + b) could be fixed by using the docksStartupHook introduced in 28e9f8b, although this wasn't documented and having to call docksStartupHook after setLayout is far from obvious. By moving the strut cache from AvoidStruts instances to a global state, b) and c) are fixed. One still has to invoke the docksStartupHook for a), and this will be addressed in the next commit.
As it now consists of a startup hook, a manage hook, an event hook and a layout modifier, and behaves erratically when any one component is not included in a user's config (which happens to be the case for all configs from xmonad-contrib 0.12 since the startup hook is a new inclusion), it's probably wise to have a single function that adds all the hooks to the config instead. NB: This will need a release notes entry anyway!
f1u77y
force-pushed
the
managedocks-global-cache
branch
from
3b373d6 to
e38fb3b
Compare
|
Any chance this could be merged? |
|
I don't really understand the issues or changes here, but if everyone involved is happy with it and you just need someone with a commit bit to click "merge" I would be happy to. I'll merge in a few days unless I hear any objections before then. |
|
@byorgey c48d81e is just an enchancement that keeps docks state in Another(e38fb3b) enchancement is described well enough in commit message, I think. (oh, and how did I not notice that this StateExtension should be persistent; will fix) |
|
OK, thanks, makes sense. I'll wait for the |
|
Oh no, no no no. c48d81e is everything but just an enhancement. That's the whole fix. And it too has a commit message that explains what is fixed and how. And no, it should definitely not be persistent. If the strut cache somehow gets out of sync, an xmonad restart is all that needed to reset it. If you make it persistent, that won't be possible. Also, there's really no point in making a cache persistent, is it? |
|
(So just to avoid any misunderstandings, I do think that it is currently ready to be merged and any additional changes may make it not ready. I did express this opinion by #80 (review)) |
|
@liskin I also think that |
|
@byorgey it's ready to be merged |
|
On Mon, Oct 24, 2016 at 6:29 PM, Bogdan Sinitsyn [email protected]
This actually trades one bug for another: currently you lose the strut brandon s allbery kf8nh sine nomine associates |
|
@geekosaur as liskin mentioned, rebuilding of the cache now can be achieved by restarting xmonad |
This makes docksStartupHook unnecessary. That is desirable because we didn't add a changelog entry about it being necessary, and 4 years after its introduction we still have users grabbing old configs and reporting (xmonad/xmonad#21 (comment)) that ManageDocks doesn't work properly. I would love to finally settle this. Full story follows: xmonad-contrib 0.12 introduced (00be056, merged in April 2013) caching to ManageDocks to avoid queryTree calls on every runLayout, which brought in several bugs. Attempts to fix these bugs in xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a breaking change in ManageDocks that required users to add docksStartupHook (or docks, after e38fb3b got merged in November 2016) to their configs for avoidStruts to still work after xmonad restart. Unfortunately this was never mentioned in the changelog nor in any compilation warning (which get discarded by xmonad --recompile anyway !!!), so as of March 2020 we still have users being bitten by this. Back in September 2016 in a discussion about other bugs introduced in 28e9f8b I suggested that we use Maybe to indicate whether the cache had been initialized and initialize it on demand when it had not. Unfortunately I wasn't sufficiently clear about what I meant and Brandon was going through some health issues, so we just got into a heated argument and ended up merging a suboptimal solution. :-( And since we're _still_ getting complaints from users every now and then, I believe it's worth dealing with even after all these years. If nothing else, let this serve as a reminder that breaking users' configs without any warning is wrong. (Oh and we should probably stop hiding xmonad.hs compilation warnings, otherwise we can't ever hope to provide smooth deprecation and upgrade paths.) Fixes: 00be056 ("Cache results from calcGap in ManageDocks") Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted") Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust") Related: xmonad#118 Related: xmonad#30 Related: xmonad#80 Related: xmonad/xmonad#21
This makes docksStartupHook unnecessary. That is desirable because we didn't add a changelog entry about it being necessary, and 4 years after its introduction we still have users grabbing old configs and reporting (xmonad/xmonad#21 (comment)) that ManageDocks doesn't work properly. I would love to finally settle this. Full story follows: xmonad-contrib 0.12 introduced (00be056, merged in April 2013) caching to ManageDocks to avoid queryTree calls on every runLayout, which brought in several bugs. Attempts to fix these bugs in xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a breaking change in ManageDocks that required users to add docksStartupHook (or docks, after e38fb3b got merged in November 2016) to their configs for avoidStruts to still work after xmonad restart. Unfortunately this was never mentioned in the changelog nor in any compilation warning (which get discarded by xmonad --recompile anyway !!!), so as of March 2020 we still have users being bitten by this. Back in September 2016 in a discussion about other bugs introduced in 28e9f8b I suggested that we use Maybe to indicate whether the cache had been initialized and initialize it on demand when it had not. Unfortunately I wasn't sufficiently clear about what I meant and Brandon was going through some health issues, so we just got into a heated argument and ended up merging a suboptimal solution. :-( And since we're _still_ getting complaints from users every now and then, I believe it's worth dealing with even after all these years. If nothing else, let this serve as a reminder that breaking users' configs without any warning is wrong. (Oh and we should probably stop hiding xmonad.hs compilation warnings, otherwise we can't ever hope to provide smooth deprecation and upgrade paths.) Fixes: 00be056 ("Cache results from calcGap in ManageDocks") Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted") Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust") Related: xmonad#118 Related: xmonad#30 Related: xmonad#80 Related: xmonad/xmonad#21
This makes docksStartupHook unnecessary. That is desirable because we didn't add a changelog entry about it being necessary, and 4 years after its introduction we still have users grabbing old configs and reporting (xmonad/xmonad#21 (comment)) that ManageDocks doesn't work properly. I would love to finally settle this. Full story follows: xmonad-contrib 0.12 introduced (00be056, merged in April 2013) caching to ManageDocks to avoid queryTree calls on every runLayout, which brought in several bugs. Attempts to fix these bugs in xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a breaking change in ManageDocks that required users to add docksStartupHook (or docks, after e38fb3b got merged in November 2016) to their configs for avoidStruts to still work after xmonad restart. Unfortunately this was never mentioned in the changelog nor in any compilation warning (which get discarded by xmonad --recompile anyway !!!), so as of March 2020 we still have users being bitten by this. Back in September 2016 in a discussion about other bugs introduced in 28e9f8b I suggested that we use Maybe to indicate whether the cache had been initialized and initialize it on demand when it had not. Unfortunately I wasn't sufficiently clear about what I meant and Brandon was going through some health issues, so we just got into a heated argument and ended up merging a suboptimal solution. :-( And since we're _still_ getting complaints from users every now and then, I believe it's worth dealing with even after all these years. If nothing else, let this serve as a reminder that breaking users' configs without any warning is wrong. (Oh and we should probably stop hiding xmonad.hs compilation warnings, otherwise we can't ever hope to provide smooth deprecation and upgrade paths.) Fixes: 00be056 ("Cache results from calcGap in ManageDocks") Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted") Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust") Related: xmonad#118 Related: xmonad#30 Related: xmonad#80 Related: xmonad/xmonad#21
This makes docksStartupHook unnecessary. That is desirable because we didn't add a changelog entry about it being necessary, and 4 years after its introduction we still have users grabbing old configs and reporting (xmonad/xmonad#21 (comment)) that ManageDocks doesn't work properly. I would love to finally settle this. Full story follows: xmonad-contrib 0.12 introduced (00be056, merged in April 2013) caching to ManageDocks to avoid queryTree calls on every runLayout, which brought in several bugs. Attempts to fix these bugs in xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a breaking change in ManageDocks that required users to add docksStartupHook (or docks, after e38fb3b got merged in November 2016) to their configs for avoidStruts to still work after xmonad restart. Unfortunately this was never mentioned in the changelog nor in any compilation warning (which get discarded by xmonad --recompile anyway !!!), so as of March 2020 we still have users being bitten by this. Back in September 2016 in a discussion about other bugs introduced in 28e9f8b I suggested that we use Maybe to indicate whether the cache had been initialized and initialize it on demand when it had not. Unfortunately I wasn't sufficiently clear about what I meant and Brandon was going through some health issues, so we just got into a heated argument and ended up merging a suboptimal solution. :-( And since we're _still_ getting complaints from users every now and then, I believe it's worth dealing with even after all these years. If nothing else, let this serve as a reminder that breaking users' configs without any warning is wrong. (Oh and we should probably stop hiding xmonad.hs compilation warnings, otherwise we can't ever hope to provide smooth deprecation and upgrade paths.) Fixes: 00be056 ("Cache results from calcGap in ManageDocks") Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted") Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust") Related: xmonad#118 Related: xmonad#30 Related: xmonad#80 Related: xmonad/xmonad#21
This makes docksStartupHook unnecessary. That is desirable because we didn't add a changelog entry about it being necessary, and 4 years after its introduction we still have users grabbing old configs and reporting (xmonad/xmonad#21 (comment)) that ManageDocks doesn't work properly. I would love to finally settle this. Full story follows: xmonad-contrib 0.12 introduced (00be056, merged in April 2013) caching to ManageDocks to avoid queryTree calls on every runLayout, which brought in several bugs. Attempts to fix these bugs in xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a breaking change in ManageDocks that required users to add docksStartupHook (or docks, after e38fb3b got merged in November 2016) to their configs for avoidStruts to still work after xmonad restart. Unfortunately this was never mentioned in the changelog nor in any compilation warning (which get discarded by xmonad --recompile anyway !!!), so as of March 2020 we still have users being bitten by this. Back in September 2016 in a discussion about other bugs introduced in 28e9f8b I suggested that we use Maybe to indicate whether the cache had been initialized and initialize it on demand when it had not. Unfortunately I wasn't sufficiently clear about what I meant and Brandon was going through some health issues, so we just got into a heated argument and ended up merging a suboptimal solution. :-( And since we're _still_ getting complaints from users every now and then, I believe it's worth dealing with even after all these years. If nothing else, let this serve as a reminder that breaking users' configs without any warning is wrong. (Oh and we should probably stop hiding xmonad.hs compilation warnings, otherwise we can't ever hope to provide smooth deprecation and upgrade paths.) Fixes: 00be056 ("Cache results from calcGap in ManageDocks") Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted") Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust") Related: xmonad#118 Related: xmonad#30 Related: xmonad#80 Related: xmonad/xmonad#21
This makes docksStartupHook unnecessary. That is desirable because we didn't add a changelog entry about it being necessary, and 4 years after its introduction we still have users grabbing old configs and reporting (xmonad/xmonad#21 (comment)) that ManageDocks doesn't work properly. I would love to finally settle this. Full story follows: xmonad-contrib 0.12 introduced (00be056, merged in April 2013) caching to ManageDocks to avoid queryTree calls on every runLayout, which brought in several bugs. Attempts to fix these bugs in xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a breaking change in ManageDocks that required users to add docksStartupHook (or docks, after e38fb3b got merged in November 2016) to their configs for avoidStruts to still work after xmonad restart. Unfortunately this was never mentioned in the changelog nor in any compilation warning (which get discarded by xmonad --recompile anyway !!!), so as of March 2020 we still have users being bitten by this. Back in September 2016 in a discussion about other bugs introduced in 28e9f8b I suggested that we use Maybe to indicate whether the cache had been initialized and initialize it on demand when it had not. Unfortunately I wasn't sufficiently clear about what I meant and Brandon was going through some health issues, so we just got into a heated argument and ended up merging a suboptimal solution. :-( And since we're _still_ getting complaints from users every now and then, I believe it's worth dealing with even after all these years. If nothing else, let this serve as a reminder that breaking users' configs without any warning is wrong. (Oh and we should probably stop hiding xmonad.hs compilation warnings, otherwise we can't ever hope to provide smooth deprecation and upgrade paths.) Fixes: 00be056 ("Cache results from calcGap in ManageDocks") Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted") Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust") Related: xmonad#118 Related: xmonad#30 Related: xmonad#80 Related: xmonad/xmonad#21
No description provided.