Repository navigation
Replies: 6 comments 4 replies
|
Hey - just opened the first PR for this: #130. Tabs engine moved into |
|
@limbo666 Cut #2 is up - PR #132 (remove ~341 lines of dead icon code from FrameManager, zero behavior change): #132 - pure deletion, builds clean. The full IconManager move was trickier than expected (half-finished extraction + unequal duplicates), so I pulled the safe dead-code cleanup out first; details in the PR. Real move to follow as its own PR if this lands. |
|
@limbo666 Cut #3 is up - PR #133 (move snap A-cluster into SnapManager, pure refactor): #133 - bonus: it eliminates the existing SnapManager-Framemanager circular dependency (those snap-preview methods were always just callbacks for SnapManager anyway). Builds clean. #132 (dead code cleanup) is still waiting on you when you get a chance. |
|
@limbo666 Separate question on the next cut (IconManager). It looks like a half-finished extraction: its AddIcon/UpdateIcon/UpdateFrameDataForIcon are dead copies (zero callers), lagging behind the live versions still in FrameManager (which have more - grayscale, custom text color, Portal branches, network fail-safe, StoreApp handling, _iconStates cache). My plan B would be: overwrite the dead copies with the live versions, merge duplicated helpers (GetUrlCustomIcon vs ExtractCustomIconFromUrl), relocate private deps (_iconStates, IsNetworkPath, BackupOrRestoreShortcut), rewire 6 external call sites, and plug LazyIconLoader into the live path. Before I move all that code around - does IconManager, in your mind, own the full icon UI lifecycle (add/update/render)? Or should it stay a lower-level icon-source/cache utility and leave UI orchestration in FrameManager? Want to align with your vision first. |
|
Yes. This was abandoned and needs to be refactored but It is quite tricky... |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Hey @limbo666 —
Been hacking on a fork of Desktop Frames + and want to start sending stuff upstream, but I keep slamming into one thing:
FrameManager.cs. Figured I'd flag it and offer to help.The deal
FrameManager.csis ~9,940 lines / 477 KB. For scale, that's ~5.7× the next-biggest file in the repo, and bigger than the next half-dozen.csfiles combined. It's basically one giant god class — ~100 methods across 15+ unrelated concerns:...all crammed into one file with a single
#region. No internal structure to speak of.Why it's a pain
Every frame-related change funnels through this one file, so any two patches — mine and yours, or two contributors' — collide constantly. It's basically the worst-case shape for git merges; keeping a fork in sync with upstream is where I hit it every single time. PRs that touch it turn into huge, scary diffs that are slow to review and easy to break. And a new contributor basically has to swallow a 10k-line file before they can safely change anything.
You've already started fixing this
Half the repo is already marked "Extracted from Framemanager for better code organization" —
FrameDataManager,FilePathUtilities,IconManager,SnapManager,LaunchEffectsManager,PortalFrameManager, and friends. So the direction's set, it just hasn't reached the core orchestrator stuff still stuck inFrameManager.What I'd do (small, safe, mechanical slices)
One extraction per PR, pure move + rename, no logic changes:
*Tab*methods) → newTabManager. Clean boundary, great first slice.IconManager.IconDragDropManager/SnapManager.FrameDataManager.LaunchEffectsManager.End state:
FrameManagershrinks to just the lifecycle/orchestration core, and changes stop all landing in the same giant file — which directly shrinks the conflict surface for everyone.Quick win while that happens: even just dropping a few
#regions in to group things by concern would make the file way easier to navigate and merge.I'm in
I can knock out #1 (the Tabs extraction) as a standalone PR — move only, no behavior changes, easy to review and bisect. Point me at whichever slice you want first and I'll get going. Or if you've got a different split in mind, I'll follow your lead.
All reactions