fullscreen-compilation: remove magic numbers
N-R-K opened this issue · 3 comments
While trying to edit some stuff I found myself not knowing what 2 or 3 meant. I think it's best to use enum instead; The names can probably be improved, but this is certainly better than what we currently have, magic numbers.
enum { FFSNone, FFSActive, FFSActual, FFSActualExit }; /* fakefullscreen */I'd supply the fix myself, but I see that the patch has evolved a bit more compared to what I have on my build. Unfortunately the changes don't look interesting enough for me to bother porting them. Here's a diff on my build if you're interested: https://dpaste.com/82NLG5QT6
Heh, I think of something else when I read FFS. Whether magic text FFSActual is more readable than magic number 2 I guess is debatable.
Style wise if you do it like this then arguably you should do it for c->isfullscreen as well?
Anyway I see the point and will take it into consideration if I end up refactoring this patch.
Heh, I think of something else when I read FFS.
Hence why the names can be improved :)
Style wise if you do it like this then arguably you should do it for c->isfullscreen as well?
Correct me if I'm wrong, but I don't think I saw anything but 0 and 1 being assigned to isfullscreen. 0 and 1 are pretty synonymous to false and true so I don't think it makes much sense to change them. At best I think one can use a bool and true/false for them instead. But that's upto the suckless guys, not really in scope for the fakefullscreen patch.
One more thing I just noticed (haven't tested it) is that c->isfullscreen = fullscreen; can probably be put inside this block.
if (fullscreen != c->isfullscreen) { // only send property change if necessary