Fix the library and inline items from the review - #894
Merged
Merged
Conversation
- MemberLine prefers a declaration whose span holds the hint, the outermost where several do, over one that is merely nearer. A same-named test in the next nested type, or an F# local named like the test, put the floor past the hint. - NextMemberLine compares indentation as columns, with tab stops of four. A tab indented body under a space indented member ended the member at its first local. - F# escapes that are malformed or not F#'s (\d, \0, \12, \e, \x4, \u12) keep the backslash as text, as fsi does, rather than making the literal unreadable. A trigraph past 255 wraps into the byte range, as fsi does. - An F# regular literal is read as the snapshot it holds, with the layout convention applied as the test library applies it, and layout-shaped content that has to be written as a regular literal is wrapped in layout of its own. The patcher called such a literal already applied, so it was never updated. - WildcardFileFinder finds nothing, rather than throwing, under a root that does not exist: an undefined Program Files variable threw out of DiffTools' static constructor. - ViewerLaunchGate.LaunchAsync runs the launch on the pool and does not capture the caller's context while it holds the gate, so a sync Launch behind it on a single threaded context no longer deadlocks. - InlineApplier answers Failed where opening the mutex or the patcher throws, rather than unwinding a viewer's loop. - Staged file names cut the test name to fit 255 bytes, and on .NET Framework the whole path to MAX_PATH. Longer names were silently not staged. - A reply on the viewer port that is not the protocol marks the port unowned and says once which variable moves DiffEngine off it, rather than being taken for an owner that silently took nothing.
This was referenced Sep 23, 2026
Merged
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
1 participant
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.
This fixes nine of the eleven "Library and inline" items from
todo.md. Each item's repro fromreview-reprosnow lives in its topic test class. Two items stay open because each needs a design decision: thedotnet testhang when a viewer is launched, and settle-by-member.MemberLinetakes the nearest same-named declarationInlinePatcherTests.ASameNamedMemberInTheNextNestedType*,InlinePatcherFsTests.AnFsLocalNamedLikeTheTestDoesNotFloorTheSearchPastTheHintNextMemberLinecompares indentation by character countIndentWidth)InlinePatcherTests.ATabIndentedLocal*,ControlSpaceIndentedLocalLeavesTheSiblingAlone"\n"…"\n", and the parser appliesStripLayoutto regular and verbatim values asSnapshotValuedoesFsCompilerRoundTripTestscases through fsi,InlinePatcherFsTests.FsPatcherDoesNotCallALayoutShapedRegularLiteralAlreadyAppliedStringLiteral.TryScanRegulartakesmalformedEscapeIsText: F# keeps the backslash, C# still rejects. A trigraph past 255 wraps, as fsi does with FS1252. All checked against fsiFsStringLiteralTests.Parse(the\0,\12and\ecases moved over fromParseRejects),InlinePatcherFsTests.FsPatcherUpdatesALiteralHoldingAnUnknownEscapeWildcardFileFinderthrows under an undefined variableWildcardFileFinderTests.AnUndefinedVariable*,ResolvingATwoLevelWildcard*ViewerLaunchGate.Launchdeadlocks behind an async oneLaunchAsyncruns the launch on the pool and awaits without capture while it holds the gate. The CLAUDE.md note onConfigureAwaitis updated to cover itViewerLaunchGateTests.SyncLaunchBehindAnAsync*InlineApplier.ApplythrowsFailedInlineApplierTests.AMutexThisProcessCannotOpenFailsTheApplyRatherThanThrowingInlineStagingTests.ALongTestNameIsStillPersistedDiffEngine_ViewerPortViewerClientUnownedTests.ANonViewerOnThePortIsReportedRatherThanTakenForAnOwnerThe full solution passes locally in Release: 2,131 passed, 22 skipped.