alt-tab-macos icon indicating copy to clipboard operation
alt-tab-macos copied to clipboard

Reorganize how window validity exceptions are checked and add ColorSlurp exception

Open zacharee opened this issue 2 years ago • 3 comments

This PR has 2 changes:

  • I noticed that checking for exceptions for when to show windows is getting a little complex and hard to read. This PR extracts checks into separate WindowCheck sub-classes and iterates over those instances, instead of creating and ORing a new function in for every exception. I also added a provision for excepting smaller-than-100x100 windows in the future.
  • ColorSlurp's picked-color dialog is meant to behave as a full window, but has an abnormal role. This adds an exception check to allow AltTab to show the ColorSlurp window.

zacharee avatar Aug 14 '22 22:08 zacharee

I also have the Ventura workaround commit here, but it should be possible to cherry-pick the other two commits.

zacharee avatar Aug 14 '22 22:08 zacharee

@zacharee thank you for sharing this work.

I would like to see what you see in this refactor, but I'm afraid I don't. The business logic is moved from simple functions into something more complex involving a class hierarchy with subclass overriding methods, instantiating classes, etc.

I think if you find the current AXUIElement.swift#isActualWindow method noisy, you could create this WindowCheck file you created here, and simply move all the various functions in there to contain the "zoo". I'm not bothered by the methods and the current volume of lines in AXUIElement to be honest, but I would understand someone wanting to cut isActualWindow out of this file. However, I don't see the value in moving from straight-forward functions that are unique and customized to each business rule, into some generic system using a class hierarchy. I find for instance that each overriden method having the whole "opts" object passed reduces clarity, versus the current implem where each business function gets passed only what it needs to discriminate the window.

lwouis avatar Aug 14 '22 23:08 lwouis

@zacharee thank you for sharing this work.

I would like to see what you see in this refactor, but I'm afraid I don't. The business logic is moved from simple functions into something more complex involving a class hierarchy with subclass overriding methods, instantiating classes, etc.

I think if you find the current AXUIElement.swift#isActualWindow method noisy, you could create this WindowCheck file you created here, and simply move all the various functions in there to contain the "zoo". I'm not bothered by the methods and the current volume of lines in AXUIElement to be honest, but I would understand someone wanting to cut isActualWindow out of this file. However, I don't see the value in moving from straight-forward functions that are unique and customized to each business rule, into some generic system using a class hierarchy. I find for instance that each overriden method having the whole "opts" object passed reduces clarity, versus the current implem where each business function gets passed only what it needs to discriminate the window.

The issue I was having with the original setup was parsing all of the nested groups of disjunctions and conjunctions and where a new exception should be added (for example, the JetBrains check was inverted vs. the rest).

There is more code, but imo now it's clearer how and when each step is checked, and adding a new exception just requires a new class and instantiating that class in the list, without needing to step through the parentheses to make sure it's placed correctly.

My inspiration was the Strategy pattern, which does have the drawback of needing more generic arguments, but the advantage of encapsulating similar functions into a predictable structure, i.e., prioritizing knowing where and when each one is used vs. knowing what each one does specifically.

Definitely up for more discussion on this.

zacharee avatar Aug 14 '22 23:08 zacharee

Closing this PR as the ColorSlurp got merged from the other PR 👍

lwouis avatar Jun 23 '23 22:06 lwouis