Sort diff changes such that docs are next to their definitions

elm-git.json

@@ -1 +1,8 @@
-{"git-dependencies":{"direct":{"https://github.com/unisonweb/ui-core":"4a9d32ef5c10cf2e755df8c158e3088ea3953641"},"indirect":{}}}
+{
+    "git-dependencies": {
+        "direct": {
+            "https://github.com/unisonweb/ui-core": "7312db2df048f55cc118b9a7d9268ae76aa5da8a"
+        },
+        "indirect": {}
+    }
+}

src/UnisonShare/BranchDiff.elm

@@ -2,7 +2,9 @@ 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
@@ -154,6 +156,114 @@ 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), indexed by the
+doc fqn
+-}
+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 the doc
+        findPairing : FQN.FQN -> Maybe { doc : ChangeLine, def : ChangeLine, docFqn : FQN.FQN, defFqn : FQN.FQN }
+        findPairing docFqn =
+            Dict.get (FQN.toString docFqn) pairings
+
+        go : List ChangeLine -> FQNSet -> 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
+                                docFqn =
+                                    head
+                                        |> ChangeLine.fullName
+                                        |> FQN.toDefinitionDoc
+                            in
+                            if FQNSet.member docFqn processed then
+                                go tail processed
+
+                            else
+                                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 (FQNSet.insert pairing.docFqn processed)
+
+                                    Nothing ->
+                                        -- Standalone item
+                                        head :: go tail processed
+    in
+    go lines FQNSet.empty
+
+
 changeLineById : ChangeLineId -> BranchDiff -> Maybe ChangeLine
 changeLineById changeLineId branchDiff =
     let

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

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_

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
+        }