search

cappuccino / cappuccino

Web Application Framework in JavaScript and Objective-J
https://cappuccino.dev/
GNU Lesser General Public License v2.1
2.2k stars 333 forks source link

new: support for releasedWhenClosed in CPWindow #3060

Closed daboe01 closed 1 month ago

daboe01 commented 4 months ago

fixes: #2177

daboe01 commented 4 months ago

@enquora hi david, can you have a look at this one, if time permits?

daboe01 commented 4 months ago

-#new +#needs-review +AppKit

cappbot commented 4 months ago

Milestone: Someday. Labels: #needs-review, AppKit. What's next? This issue is pending an architectural or implementation design decision and should be discussed or voted on.

enquora commented 4 months ago

I can look at it in detail next week, if you have concerns. Quick thoughts:

I will need some time to grok if we're covering all possible observers. You have a more comprehensive understanding of this than I do - if you're comfortable all conditions are covered and my next two comments are not a concern, and want to merge before next week, I'm ok with that.

The view hierarchy is called recursively. Is there risk of a hierarchy so deep it reaches stack limits? I have a patch adding view controllers to the responder chain, but it walks up the view hierarchy rather than down. Only the most recent Safari supports tail call recursion. It's a work-in-progress for Firefox and Chrome seems uninterested. I don't know whether this is all moot or of real concern.

I do have an antipathy to if statements which use indenting alone and omit braces - prefer seeing explicit block delimiters. Having said that, I know this has seen significant usage throughout the codebase's history. If explicitly part of our style guide, I'll get over it. I do find it an impediment to reliably scanning code though.