Code review comment for lp:~brandontschaefer/unity/disable-switcher-when-wall-active

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Hi,

10 + return (!(results.size() == 1 &&
11 + results[0]->GetIconType() == AbstractLauncherIcon::IconType::TYPE_DESKTOP) &&
12 + !WindowManager::Default()->IsWallActive() &&
13 + !results.empty());

Can we please change this to be a bit more sane? First of all, you're checking both "results.size() == 1" and then later checking "!results.empty()". If the first is true, the second will always be true. I'm also not a fn of multi-line code that requires you to write "(!(".

71 + self.workspace.switch_to(0)
72 + sleep(1)
73 + self.keyboard.press("Ctrl+Alt+Right")
74 + sleep(1)
75 + self.keybinding_hold_part_then_tap("switcher/reveal_normal")
76 + self.addCleanup(self.switcher.terminate)
77 + self.keyboard.release("Ctrl+Alt")

This will leave keys in a pressed state. Please add cleanup actions to release the Ctrl+Alt+Right keys.

Other than that, looks good!

review: Needs Fixing (quality)

« Back to merge proposal