Split the file tree row and git changes header - #69
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A sweep for oversized functions across the whole project found four composables far over the 60-line threshold that were real complexity rather than incidental length:
AslFileTreeRow(:designsystem)GitChangesHeaderGitActionsOverflowMenuRemotesViewWhat changed
FileTree.kt—AslFileTreeRowkeeps the gesture detection, the menu-anchor arithmetic and its threeremembers (that coordination is why the composable exists) and delegates the rest:FileTreeRowContent(node, expanded, selected, selectDirectories, callbacks, modifier)— the visual row.RowScope.FileTreeRowLabel(...)— the name and git-status portion.BoxScope.FileTreeRowMenu(...)— the context menu.FileTreeRowCallbacks— a small holder grouping the three row callbacks.GitPanelScreen.kt—GitChangesHeader's status/progress area and chip row became separate composables. TheLaunchedEffectauto-dismissingstatusMessagestayed put, since moving it into a child would change how it is keyed.RemotesViewwas split too.Behavior
No behavior change. No state moved, nothing became or stopped being
remembered, and composition order is preserved.FileTree.kt's gesture handling and the menu-position arithmetic that decides whether the context menu opens upward near a screen edge are unchanged.Scope — what was deliberately left long
Not every long function should be split. Left alone on purpose:
data/templates/*.kt—renderWrapper(81),renderAppBuild(70),renderAppBuildKts(68),renderAppBuildGroovy(64) and the other template files build Gradle/manifest file content as strings. Their length is the shape of the file they emit; fragmenting them destroys the ability to see the output at a glance.CodeEditorView.onKeyDown(66) — a dispatchwhenover key codes.TerminalEmulatorView.GitActionsOverflowMenuwas assessed and kept whole — there was no seam that would not have produced meaningless fragments. One honest baseline entry beats shredding a coherent function.Tests
No new tests. Pure composable moves with no logic to assert on.
Verification
./gradlew :designsystem:compileDebugKotlin :feature:git:testDebugUnitTest :app:compileDebugKotlin— pass../gradlew detekt— passes.Modifier-extension composables remain, and that theRowScope/BoxScopeextensions are intact.Baseline
Both baselines regenerated in their own commit — removals only, zero additions.
:designsystem125 → 123,:feature:git25 → 21. Project total 363 → 357.One removal needs explaining:
LongParameterListforAslEditorToolbar, which this PR never touched. It is untouched on this branch and now has 7 parameters because #57 removed itsonOverflowSelect. The baseline was not regenerated then, so that improvement went unrecorded — the second such stale entry traced back to that PR.Note
The first attempt at this split made
FileTreeRowContenta@Composableextension onModifier, called asModifier.width(...).padding(...).FileTreeRowContent(...). It compiled and rendered correctly, but it inverts the Compose convention — modifier extensions returnModifier, composables emit UI — and at the call site it reads as a modifier chain whose result is discarded. It was reworked into a plain composable takingmodifieras its last parameter. TheRowScope/BoxScopeextensions were kept, since layout-scope extensions are the correct idiom for children needingweightoralign.