From 49275fe46102a8ccc2d8b2bb3216fef2a106ea82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20H=C3=B8jberg?= Date: Fri, 6 Feb 2026 16:07:00 -0500 Subject: [PATCH 1/2] Sort docs in diffs Sort the changes in a diff such that changed definition docs appear just before their associated definitions --- elm-git.json | 9 +- src/UnisonShare/BranchDiff.elm | 123 ++++++++++++++++++ src/UnisonShare/BranchDiff/ChangeLine.elm | 15 +++ .../Page/ProjectContributionChangesPage.elm | 7 +- tests/UnisonShare/BranchDiffTests.elm | 93 +++++++++++++ 5 files changed, 245 insertions(+), 2 deletions(-) diff --git a/elm-git.json b/elm-git.json index ba22c435..bf22a01d 100644 --- a/elm-git.json +++ b/elm-git.json @@ -1 +1,8 @@ -{"git-dependencies":{"direct":{"https://github.com/unisonweb/ui-core":"4a9d32ef5c10cf2e755df8c158e3088ea3953641"},"indirect":{}}} \ No newline at end of file +{ + "git-dependencies": { + "direct": { + "https://github.com/unisonweb/ui-core": "d7da320eeec137411438689a6b9dcfe9d91b6ba5" + }, + "indirect": {} + } +} \ No newline at end of file diff --git a/src/UnisonShare/BranchDiff.elm b/src/UnisonShare/BranchDiff.elm index b222f402..d0655d5c 100644 --- a/src/UnisonShare/BranchDiff.elm +++ b/src/UnisonShare/BranchDiff.elm @@ -3,10 +3,12 @@ module UnisonShare.BranchDiff exposing (..) import Code.BranchRef as BranchRef exposing (BranchRef) import Code.FullyQualifiedName as FQN import Code.Hash as Hash exposing (Hash) +import Dict exposing (Dict) import Json.Decode as Decode exposing (Decoder) import Json.Decode.Pipeline exposing (required, requiredAt) import List.Extra as ListE import Maybe.Extra as MaybeE +import Set exposing (Set) import UnisonShare.BranchDiff.ChangeLine as ChangeLine exposing (ChangeLine(..)) import UnisonShare.BranchDiff.ChangeLineId exposing (ChangeLineId) import UnisonShare.BranchDiff.LibDep as LibDep exposing (LibDep) @@ -154,6 +156,127 @@ condense changeLines = List.foldl f [] changeLines +type alias DocDefPairings = + Dict + String + { doc : ChangeLine + , def : ChangeLine + , docFqn : FQN.FQN + , defFqn : FQN.FQN + } + + +{-| all doc/definition pairs that exist in the list (shallow) +-} +toDocDefPairings : List ChangeLine -> DocDefPairings +toDocDefPairings lines = + let + ( docs, defs ) = + List.foldr + (\cl ( docAcc, defAcc ) -> + case cl of + Namespace _ -> + ( docAcc, defAcc ) + + _ -> + if ChangeLine.isDefinitionDoc cl then + ( cl :: docAcc, defAcc ) + + else + ( docAcc, cl :: defAcc ) + ) + ( [], [] ) + lines + + pair doc = + let + docFqn = + ChangeLine.fullName doc + in + defs + |> ListE.find (ChangeLine.fullName >> FQN.isDefinitionDocOf docFqn) + |> Maybe.map + (\def -> + ( FQN.toString docFqn + , { doc = doc + , def = def + , docFqn = docFqn + , defFqn = ChangeLine.fullName def + } + ) + ) + in + docs + |> List.filterMap pair + |> Dict.fromList + + +{-| Sort change lines so that definitions and their corresponding .doc changes +appear next to each other, with the doc appearing before the definition. +For example, if both MyType and MyType.doc changed, they will be adjacent +in the sorted list with MyType.doc appearing first. +-} +sortWithDocs : List ChangeLine -> List ChangeLine +sortWithDocs lines = + let + pairings = + toDocDefPairings lines + + -- Look up pairing by either doc or def FQN + findPairing : FQN.FQN -> Maybe { doc : ChangeLine, def : ChangeLine, docFqn : FQN.FQN, defFqn : FQN.FQN } + findPairing fqn = + let + key = + if FQN.endsWith "doc" fqn then + FQN.toString fqn + + else + FQN.toString (FQN.snoc fqn "doc") + in + Dict.get key pairings + + go : List ChangeLine -> Set String -> List ChangeLine + go remaining processed = + case remaining of + [] -> + [] + + head :: tail -> + case head of + Namespace ns -> + Namespace { ns | lines = sortWithDocs ns.lines } :: go tail processed + + _ -> + let + headFqn = + ChangeLine.fullName head + + -- Always use the doc name for tracking + docName = + if ChangeLine.isDefinitionDoc head then + FQN.toString headFqn + + else + FQN.toString (FQN.snoc headFqn "doc") + in + if Set.member docName processed then + go tail processed + + else + case findPairing headFqn of + Just pairing -> + -- This item is part of a pair, output both and mark doc as processed + pairing.doc + :: pairing.def + :: go tail (Set.insert (FQN.toString pairing.docFqn) processed) + + Nothing -> + -- Standalone item + head :: go tail processed + in + go lines Set.empty + + changeLineById : ChangeLineId -> BranchDiff -> Maybe ChangeLine changeLineById changeLineId branchDiff = let diff --git a/src/UnisonShare/BranchDiff/ChangeLine.elm b/src/UnisonShare/BranchDiff/ChangeLine.elm index 0b4d37c5..95e6429e 100644 --- a/src/UnisonShare/BranchDiff/ChangeLine.elm +++ b/src/UnisonShare/BranchDiff/ChangeLine.elm @@ -90,6 +90,21 @@ isPropagated cl = False +isDoc : ChangeLine -> Bool +isDoc cl = + case definitionType cl of + Just DefinitionType.Doc -> + True + + _ -> + False + + +isDefinitionDoc : ChangeLine -> Bool +isDefinitionDoc cl = + isDoc cl && FQN.isDefinitionDoc (fullName cl) + + shouldBeCollapsedByDefault : ChangeLine -> Bool shouldBeCollapsedByDefault changeLine = case changeLine of diff --git a/src/UnisonShare/Page/ProjectContributionChangesPage.elm b/src/UnisonShare/Page/ProjectContributionChangesPage.elm index 0f455f2b..208c1ed2 100644 --- a/src/UnisonShare/Page/ProjectContributionChangesPage.elm +++ b/src/UnisonShare/Page/ProjectContributionChangesPage.elm @@ -124,7 +124,12 @@ update appContext projectRef contribRef msg model = ( { model | branchDiff = BranchDiffState.Computed - { bd | lines = BranchDiff.condense bd.lines } + { bd + | lines = + bd.lines + |> BranchDiff.condense + |> BranchDiff.sortWithDocs + } , toggledChangeLines = toggledChangeLines } , cmd_ diff --git a/tests/UnisonShare/BranchDiffTests.elm b/tests/UnisonShare/BranchDiffTests.elm index cad60437..a5ab564d 100644 --- a/tests/UnisonShare/BranchDiffTests.elm +++ b/tests/UnisonShare/BranchDiffTests.elm @@ -4,6 +4,7 @@ import Code.BranchRef as BranchRef import Code.Definition.Reference as Reference import Code.FullyQualifiedName as FQN import Code.Hash as Hash +import Code.Syntax as Syntax import Code.Syntax.SyntaxSegment as SyntaxSegment import Expect import List.Nonempty as NEL @@ -162,3 +163,95 @@ diff = } in DefinitionDiff.Diff diffDetails + + +sortWithDocs : Test +sortWithDocs = + describe "BranchDiff.sortWithDocs" + [ test "pairs definitions with their .doc changes (doc before definition) regardless of input order" <| + \_ -> + let + -- Definitions with docs in various orders + aDoc = + addedChangeLine (FQN.fromString "a.doc") (FQN.fromString "a.doc") DefinitionType.Doc + + a = + addedChangeLine (FQN.fromString "a") (FQN.fromString "a") DefinitionType.Term + + b = + addedChangeLine (FQN.fromString "b") (FQN.fromString "b") DefinitionType.Term + + bDoc = + addedChangeLine (FQN.fromString "b.doc") (FQN.fromString "b.doc") DefinitionType.Doc + + -- Definition without doc + c = + addedChangeLine (FQN.fromString "c") (FQN.fromString "c") DefinitionType.Term + + -- Standalone doc (no corresponding definition) + dDoc = + addedChangeLine (FQN.fromString "d.doc") (FQN.fromString "d.doc") DefinitionType.Doc + + -- Input: doc first, def later, def first, doc later, def without doc, standalone doc + input = + [ aDoc, b, c, a, dDoc, bDoc ] + + -- Expected: pairs together with doc first (paired when encountered) + expected = + [ aDoc, a, bDoc, b, c, dDoc ] + + result = + BranchDiff.sortWithDocs input + in + Expect.equal expected result + , test "recursively sorts within namespaces" <| + \_ -> + let + innerDef = + addedChangeLine (FQN.fromString "data.List.map") (FQN.fromString "List.map") DefinitionType.Term + + innerDoc = + addedChangeLine (FQN.fromString "data.List.map.doc") (FQN.fromString "List.map.doc") DefinitionType.Doc + + innerOther = + addedChangeLine (FQN.fromString "data.List.filter") (FQN.fromString "List.filter") DefinitionType.Term + + namespace = + ChangeLine.Namespace + { name = FQN.fromString "data.List" + , lines = [ innerDef, innerOther, innerDoc ] + } + + expectedNamespace = + ChangeLine.Namespace + { name = FQN.fromString "data.List" + , lines = [ innerDoc, innerDef, innerOther ] + } + + input = + [ namespace ] + + expected = + [ expectedNamespace ] + + result = + BranchDiff.sortWithDocs input + in + Expect.equal expected result + ] + + +addedChangeLine : FQN.FQN -> FQN.FQN -> DefinitionType.DefinitionType -> ChangeLine.ChangeLine +addedChangeLine fullName shortName type_ = + let + source = + Syntax.fromNEL (NEL.singleton (SyntaxSegment.SyntaxSegment SyntaxSegment.TextLiteral "test")) + in + ChangeLine.Added + type_ + { hash = Hash.unsafeFromString "#testHash" + , shortName = shortName + , fullName = fullName + , ref = Reference.fromFQN (DefinitionType.toReferenceConstructor type_) fullName + , source = source + } From 77de5134dc71e5f67170fea67171126688030e7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20H=C3=B8jberg?= Date: Fri, 6 Feb 2026 16:21:03 -0500 Subject: [PATCH 2/2] BranchDiff.sortWithDocs: Use FQNSet --- elm-git.json | 2 +- src/UnisonShare/BranchDiff.elm | 43 ++++++++++++---------------------- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/elm-git.json b/elm-git.json index bf22a01d..a9d92598 100644 --- a/elm-git.json +++ b/elm-git.json @@ -1,7 +1,7 @@ { "git-dependencies": { "direct": { - "https://github.com/unisonweb/ui-core": "d7da320eeec137411438689a6b9dcfe9d91b6ba5" + "https://github.com/unisonweb/ui-core": "7312db2df048f55cc118b9a7d9268ae76aa5da8a" }, "indirect": {} } diff --git a/src/UnisonShare/BranchDiff.elm b/src/UnisonShare/BranchDiff.elm index d0655d5c..902d5ca1 100644 --- a/src/UnisonShare/BranchDiff.elm +++ b/src/UnisonShare/BranchDiff.elm @@ -2,13 +2,13 @@ module UnisonShare.BranchDiff exposing (..) import Code.BranchRef as BranchRef exposing (BranchRef) import Code.FullyQualifiedName as FQN +import Code.FullyQualifiedNameSet as FQNSet exposing (FQNSet) import Code.Hash as Hash exposing (Hash) import Dict exposing (Dict) import Json.Decode as Decode exposing (Decoder) import Json.Decode.Pipeline exposing (required, requiredAt) import List.Extra as ListE import Maybe.Extra as MaybeE -import Set exposing (Set) import UnisonShare.BranchDiff.ChangeLine as ChangeLine exposing (ChangeLine(..)) import UnisonShare.BranchDiff.ChangeLineId exposing (ChangeLineId) import UnisonShare.BranchDiff.LibDep as LibDep exposing (LibDep) @@ -166,7 +166,8 @@ type alias DocDefPairings = } -{-| all doc/definition pairs that exist in the list (shallow) +{-| all doc/definition pairs that exist in the list (shallow), indexed by the +doc fqn -} toDocDefPairings : List ChangeLine -> DocDefPairings toDocDefPairings lines = @@ -222,20 +223,12 @@ sortWithDocs lines = pairings = toDocDefPairings lines - -- Look up pairing by either doc or def FQN + -- Look up pairing by the doc findPairing : FQN.FQN -> Maybe { doc : ChangeLine, def : ChangeLine, docFqn : FQN.FQN, defFqn : FQN.FQN } - findPairing fqn = - let - key = - if FQN.endsWith "doc" fqn then - FQN.toString fqn - - else - FQN.toString (FQN.snoc fqn "doc") - in - Dict.get key pairings + findPairing docFqn = + Dict.get (FQN.toString docFqn) pairings - go : List ChangeLine -> Set String -> List ChangeLine + go : List ChangeLine -> FQNSet -> List ChangeLine go remaining processed = case remaining of [] -> @@ -248,33 +241,27 @@ sortWithDocs lines = _ -> let - headFqn = - ChangeLine.fullName head - - -- Always use the doc name for tracking - docName = - if ChangeLine.isDefinitionDoc head then - FQN.toString headFqn - - else - FQN.toString (FQN.snoc headFqn "doc") + docFqn = + head + |> ChangeLine.fullName + |> FQN.toDefinitionDoc in - if Set.member docName processed then + if FQNSet.member docFqn processed then go tail processed else - case findPairing headFqn of + case findPairing docFqn of Just pairing -> -- This item is part of a pair, output both and mark doc as processed pairing.doc :: pairing.def - :: go tail (Set.insert (FQN.toString pairing.docFqn) processed) + :: go tail (FQNSet.insert pairing.docFqn processed) Nothing -> -- Standalone item head :: go tail processed in - go lines Set.empty + go lines FQNSet.empty changeLineById : ChangeLineId -> BranchDiff -> Maybe ChangeLine