⅄ trunk → 24-06-24-todo-dependents

Author: mitchellwrosen

.github/workflows/ci.yaml

@@ -5,16 +5,10 @@ defaults:
     shell: bash
 
 on:
-  # Build on every pull request (and new PR commit)
+  # Run on the post-merge result of every PR commit
   pull_request:
-  # Build on new pushes to trunk (E.g. Merge commits)
-  # Without the branch filter, each commit on a branch with a PR is triggered twice.
-  # See: https://github.community/t/how-to-trigger-an-action-on-push-or-pull-request-but-not-both/16662
+  # Build on the pre-merge result of every branch commit
   push:
-    branches:
-      - trunk
-    tags:
-      - release/*
   workflow_dispatch:
 
 env:
@@ -270,6 +264,14 @@ jobs:
           ${{env.transcripts}}
           # Fail if any transcripts cause git diffs.
           git diff --ignore-cr-at-eol --exit-code unison-src/transcripts
+      - name: docs.to-html
+        if: steps.cache-transcript-test-results.outputs.cache-hit != 'true'
+        run: |
+          ${{env.ucm}} transcript unison-src/transcripts-manual/docs.to-html.md
+          # Fail if the output or generated docs differ.
+          git diff --ignore-cr-at-eol --exit-code \
+            unison-src/transcripts-manual/docs.to-html.output.md \
+            unison-src/transcripts-manual/docs.to-html
       - name: mark transcripts as passing
         if: steps.cache-transcript-test-results.outputs.cache-hit != 'true'
         run: |
@@ -417,7 +419,7 @@ jobs:
   build-jit-binary:
     name: build jit binary
     needs: generate-jit-source
-    uses: ./.github/workflows/ci-build-jit-binary.yaml    
+    uses: ./.github/workflows/ci-build-jit-binary.yaml
 
   test-jit:
     name: test jit

.github/workflows/update-transcripts.yaml

@@ -36,6 +36,9 @@ jobs:
           stack exec unison transcript unison-src/transcripts-manual/rewrites.md
       - name: transcripts
         run: stack exec transcripts
+      - name: docs.to-html
+        run: |
+          stack exec unison transcript unison-src/transcripts-manual/docs.to-html.md
       - name: save transcript changes
         uses: stefanzweifel/git-auto-commit-action@v5
         with:

parser-typechecker/src/Unison/Codebase/Path.hs

@@ -43,7 +43,7 @@ module Unison.Codebase.Path
     isRoot,
     isRoot',
 
-    -- * things that could be replaced with `Convert` instances
+    -- * conversions
     absoluteToPath',
     fromList,
     fromName,
@@ -76,8 +76,6 @@ module Unison.Codebase.Path
     -- * things that could be replaced with `Snoc` instances
     snoc,
     unsnoc,
-    -- This should be moved to a common util module, or we could use the 'witch' package.
-    Convert (..),
   )
 where
 
@@ -93,14 +91,19 @@ import Data.Sequence qualified as Seq
 import Data.Text qualified as Text
 import GHC.Exts qualified as GHC
 import Unison.HashQualified' qualified as HQ'
-import Unison.Name (Convert (..), Name)
+import Unison.Name (Name)
 import Unison.Name qualified as Name
 import Unison.NameSegment (NameSegment)
 import Unison.Prelude hiding (empty, toList)
 import Unison.Syntax.Name qualified as Name (toText, unsafeParseText)
 import Unison.Util.List qualified as List
 
--- `Foo.Bar.baz` becomes ["Foo", "Bar", "baz"]
+-- | A `Path` is an internal structure representing some namespace in the codebase.
+--
+--  @Foo.Bar.baz@ becomes @["Foo", "Bar", "baz"]@.
+--
+--  __NB__:  This shouldn’t be exposed outside of this module (prefer`Path'`, `Absolute`, or `Relative`), but it’s
+--   currently used pretty widely. Such usage should be replaced when encountered.
 newtype Path = Path {toSeq :: Seq NameSegment}
   deriving stock (Eq, Ord)
   deriving newtype (Semigroup, Monoid)
@@ -112,10 +115,13 @@ instance GHC.IsList Path where
   toList (Path segs) = Foldable.toList segs
   fromList = Path . Seq.fromList
 
+-- | A namespace path that starts from the root.
 newtype Absolute = Absolute {unabsolute :: Path} deriving (Eq, Ord)
 
+-- | A namespace path that doesn’t necessarily start from the root.
 newtype Relative = Relative {unrelative :: Path} deriving (Eq, Ord)
 
+-- | A namespace that may be either absolute or relative, This is the most general type that should be used.
 newtype Path' = Path' {unPath' :: Either Absolute Relative}
   deriving (Eq, Ord)
 
@@ -534,34 +540,3 @@ instance Resolve Absolute HQSplit HQSplitAbsolute where
 instance Resolve Absolute Path' Absolute where
   resolve _ (AbsolutePath' a) = a
   resolve a (RelativePath' r) = resolve a r
-
-instance Convert Absolute Path where convert = unabsolute
-
-instance Convert Absolute Path' where convert = absoluteToPath'
-
-instance Convert Absolute Text where convert = toText' . absoluteToPath'
-
-instance Convert Relative Text where convert = toText . unrelative
-
-instance Convert Absolute String where convert = Text.unpack . convert
-
-instance Convert Relative String where convert = Text.unpack . convert
-
-instance Convert [NameSegment] Path where convert = fromList
-
-instance Convert Path [NameSegment] where convert = toList
-
-instance Convert HQSplit (HQ'.HashQualified Path) where convert = unsplitHQ
-
-instance Convert HQSplit' (HQ'.HashQualified Path') where convert = unsplitHQ'
-
-instance Convert Name Split where
-  convert = splitFromName
-
-instance Convert (path, NameSegment) (path, HQ'.HQSegment) where
-  convert (path, name) =
-    (path, HQ'.fromName name)
-
-instance (Convert path0 path1) => Convert (path0, name) (path1, name) where
-  convert =
-    over _1 convert

parser-typechecker/src/Unison/PrintError.hs

@@ -126,6 +126,10 @@ styleAnnotated sty a = (,sty) <$> rangeForAnnotated a
 style :: s -> String -> Pretty (AnnotatedText s)
 style sty str = Pr.lit . AT.annotate sty $ fromString str
 
+-- | Applies the color highlighting for `Code`, but also quotes the code, to separate it from the containing context.
+quoteCode :: String -> Pretty ColorText
+quoteCode = Pr.backticked . style Code
+
 stylePretty :: Color -> Pretty ColorText -> Pretty ColorText
 stylePretty = Pr.map . AT.annotate
 
@@ -1366,31 +1370,31 @@ renderParseErrors s = \case
             <> style ErrorSite (fromString open)
             <> ".\n\n"
             <> excerpt
-        L.InvalidWordyId _id ->
+        L.ReservedWordyId id ->
           Pr.lines
-            [ "This identifier isn't valid syntax: ",
+            [ "The identifier " <> quoteCode id <> " used here is a reserved keyword: ",
               "",
               excerpt,
-              "Here's a few examples of valid syntax: "
-                <> style Code "abba1', snake_case, Foo.zoink!, 🌻"
-            ]
-        L.ReservedWordyId _id ->
-          Pr.lines
-            [ "The identifier used here isn't allowed to be a reserved keyword: ",
-              "",
-              excerpt
+              Pr.wrap $
+                "You can avoid this problem either by renaming the identifier or wrapping it in backticks (like "
+                  <> style Code ("`" <> id <> "`")
+                  <> ")."
             ]
-        L.InvalidSymbolyId _id ->
+        L.InvalidSymbolyId id ->
           Pr.lines
-            [ "This infix identifier isn't valid syntax: ",
+            [ "The infix identifier " <> quoteCode id <> " isn’t valid syntax: ",
               "",
               excerpt,
-              "Here's a few valid examples: "
-                <> style Code "++, Float./, `List.map`"
+              "Here are a few valid examples: "
+                <> quoteCode "++"
+                <> ", "
+                <> quoteCode "Float./"
+                <> ", and "
+                <> quoteCode "List.map"
             ]
-        L.ReservedSymbolyId _id ->
+        L.ReservedSymbolyId id ->
           Pr.lines
-            [ "This identifier is reserved by Unison and can't be used as an operator: ",
+            [ "The identifier " <> quoteCode id <> " is reserved by Unison and can't be used as an operator: ",
               "",
               excerpt
             ]
@@ -1444,11 +1448,12 @@ renderParseErrors s = \case
               "",
               excerpt,
               Pr.wrap $
-                "I was expecting some digits after the '.',"
-                  <> "for example: "
-                  <> style Code (n <> "0")
+                "I was expecting some digits after the "
+                  <> quoteCode "."
+                  <> ", for example: "
+                  <> quoteCode (n <> "0")
                   <> "or"
-                  <> Pr.group (style Code (n <> "1e37") <> ".")
+                  <> Pr.group (quoteCode (n <> "1e37") <> ".")
             ]
         L.MissingExponent n ->
           Pr.lines
@@ -1458,7 +1463,7 @@ renderParseErrors s = \case
               Pr.wrap $
                 "I was expecting some digits for the exponent,"
                   <> "for example: "
-                  <> Pr.group (style Code (n <> "37") <> ".")
+                  <> Pr.group (quoteCode (n <> "37") <> ".")
             ]
         L.TextLiteralMissingClosingQuote _txt ->
           Pr.lines
@@ -1474,7 +1479,7 @@ renderParseErrors s = \case
               "",
               "I only know about the following escape characters:",
               "",
-              let s ch = style Code (fromString $ "\\" <> [ch])
+              let s ch = quoteCode (fromString $ "\\" <> [ch])
                in Pr.indentN 2 $ intercalateMap "," s (fst <$> L.escapeChars)
             ]
         L.LayoutError ->
@@ -1705,7 +1710,7 @@ renderParseErrors s = \case
           let msg =
                 mconcat
                   [ "This looks like the start of an expression here but I was expecting a binding.",
-                    "\nDid you mean to use a single " <> style Code ":",
+                    "\nDid you mean to use a single " <> quoteCode ":",
                     " here for a type signature?",
                     "\n\n",
                     tokenAsErrorSite s t

parser-typechecker/src/Unison/Syntax/TermPrinter.hs

@@ -459,7 +459,7 @@ pretty0
                     go tm = goNormal 10 tm
                 PP.hang kw <$> fmap PP.lines (traverse go rs)
               (Bytes' bs, _) ->
-                pure $ fmt S.BytesLiteral "0xs" <> PP.shown (Bytes.fromWord8s (map fromIntegral bs))
+                pure $ PP.group $ fmt S.BytesLiteral "0xs" <> PP.shown (Bytes.fromWord8s (map fromIntegral bs))
               BinaryAppsPred' apps lastArg -> do
                 prettyLast <- pretty0 (ac 3 Normal im doc) lastArg
                 prettyApps <- binaryApps apps prettyLast

scripts/check.sh

@@ -6,4 +6,5 @@ true \
   && stack exec transcripts \
   && stack exec unison transcript unison-src/transcripts-round-trip/main.md \
   && stack exec unison transcript unison-src/transcripts-manual/rewrites.md \
+  && stack exec unison transcript unison-src/transcripts-manual/docs.to-html.md \
   && stack exec cli-integration-tests

unison-cli/src/Unison/Cli/MonadUtils.hs

@@ -481,15 +481,15 @@ updateRoot new reason =
 getTermsAt :: (Path.Absolute, HQ'.HQSegment) -> Cli (Set Referent)
 getTermsAt path = do
   rootBranch0 <- getRootBranch0
-  pure (BranchUtil.getTerm (Path.convert path) rootBranch0)
+  pure (BranchUtil.getTerm (first Path.unabsolute path) rootBranch0)
 
 ------------------------------------------------------------------------------------------------------------------------
 -- Getting types
 
 getTypesAt :: (Path.Absolute, HQ'.HQSegment) -> Cli (Set TypeReference)
 getTypesAt path = do
   rootBranch0 <- getRootBranch0
-  pure (BranchUtil.getType (Path.convert path) rootBranch0)
+  pure (BranchUtil.getType (first Path.unabsolute path) rootBranch0)
 
 ------------------------------------------------------------------------------------------------------------------------
 -- Getting patches

unison-cli/src/Unison/Cli/ProjectUtils.hs

@@ -124,8 +124,8 @@ justTheIds x =
   ProjectAndBranch x.project.projectId x.branch.branchId
 
 justTheIds' :: Sqlite.ProjectBranch -> ProjectAndBranch ProjectId ProjectBranchId
-justTheIds' x =
-  ProjectAndBranch x.projectId x.branchId
+justTheIds' branch =
+  ProjectAndBranch branch.projectId branch.branchId
 
 justTheNames :: ProjectAndBranch Sqlite.Project Sqlite.ProjectBranch -> ProjectAndBranch ProjectName ProjectBranchName
 justTheNames x =

unison-cli/src/Unison/Codebase/Editor/HandleInput.hs

@@ -66,6 +66,7 @@ import Unison.Codebase.Editor.HandleInput.FindAndReplace (handleStructuredFindI,
 import Unison.Codebase.Editor.HandleInput.FormatFile qualified as Format
 import Unison.Codebase.Editor.HandleInput.InstallLib (handleInstallLib)
 import Unison.Codebase.Editor.HandleInput.Load (EvalMode (Sandboxed), evalUnisonFile, handleLoad, loadUnisonFile)
+import Unison.Codebase.Editor.HandleInput.Ls (handleLs)
 import Unison.Codebase.Editor.HandleInput.Merge2 (handleMerge)
 import Unison.Codebase.Editor.HandleInput.MoveAll (handleMoveAll)
 import Unison.Codebase.Editor.HandleInput.MoveBranch (doMoveBranch)
@@ -347,7 +348,7 @@ loop e = do
                   Left hash -> (,WhichBranchEmptyHash hash) <$> Cli.resolveShortCausalHash hash
                   Right path' -> do
                     absPath <- ProjectUtils.branchRelativePathToAbsolute path'
-                    let srcp = Path.convert absPath
+                    let srcp = Path.AbsolutePath' absPath
                     srcb <- Cli.expectBranchAtPath' srcp
                     pure (srcb, WhichBranchEmptyPath srcp)
               description <- inputDescription input
@@ -465,7 +466,7 @@ loop e = do
               Cli.respond $ Output.MarkdownOut (Text.intercalate "\n---\n" mdText)
             DocsToHtmlI namespacePath' sourceDirectory -> do
               Cli.Env {codebase, sandboxedRuntime} <- ask
-              absPath <- Cli.resolvePath' namespacePath'
+              absPath <- ProjectUtils.branchRelativePathToAbsolute namespacePath'
               branch <- liftIO $ Codebase.getBranchAtPath codebase absPath
               _evalErrs <- liftIO $ (Backend.docsInBranchToHtmlFiles sandboxedRuntime codebase branch sourceDirectory)
               pure ()
@@ -487,11 +488,11 @@ loop e = do
                       hqLength <- Cli.runTransaction Codebase.hashLength
                       pure (DeleteNameAmbiguous hqLength name srcTerms Set.empty)
               dest <- Cli.resolveSplit' dest'
-              destTerms <- Cli.getTermsAt (Path.convert dest)
+              destTerms <- Cli.getTermsAt (HQ'.NameOnly <$> dest)
               when (not (Set.null destTerms)) do
                 Cli.returnEarly (TermAlreadyExists dest' destTerms)
               description <- inputDescription input
-              Cli.stepAt description (BranchUtil.makeAddTermName (Path.convert dest) srcTerm)
+              Cli.stepAt description (BranchUtil.makeAddTermName (first Path.unabsolute dest) srcTerm)
               Cli.respond Success
             AliasTypeI src' dest' -> do
               src <- traverseOf _Right Cli.resolveSplit' src'
@@ -510,11 +511,11 @@ loop e = do
                       hqLength <- Cli.runTransaction Codebase.hashLength
                       pure (DeleteNameAmbiguous hqLength name Set.empty srcTypes)
               dest <- Cli.resolveSplit' dest'
-              destTypes <- Cli.getTypesAt (Path.convert dest)
+              destTypes <- Cli.getTypesAt (HQ'.NameOnly <$> dest)
               when (not (Set.null destTypes)) do
                 Cli.returnEarly (TypeAlreadyExists dest' destTypes)
               description <- inputDescription input
-              Cli.stepAt description (BranchUtil.makeAddTypeName (Path.convert dest) srcType)
+              Cli.stepAt description (BranchUtil.makeAddTypeName (first Path.unabsolute dest) srcType)
               Cli.respond Success
 
             -- this implementation will happily produce name conflicts,
@@ -616,9 +617,9 @@ loop e = do
               guidPath <- Cli.resolveSplit' (authorPath' |> NameSegment.guidSegment)
               Cli.stepManyAt
                 description
-                [ BranchUtil.makeAddTermName (Path.convert authorPath) (d authorRef),
-                  BranchUtil.makeAddTermName (Path.convert copyrightHolderPath) (d copyrightHolderRef),
-                  BranchUtil.makeAddTermName (Path.convert guidPath) (d guidRef)
+                [ BranchUtil.makeAddTermName (first Path.unabsolute authorPath) (d authorRef),
+                  BranchUtil.makeAddTermName (first Path.unabsolute copyrightHolderPath) (d copyrightHolderRef),
+                  BranchUtil.makeAddTermName (first Path.unabsolute guidPath) (d guidRef)
                 ]
               currentPath <- Cli.getCurrentPath
               finalBranch <- Cli.getCurrentBranch0
@@ -687,21 +688,7 @@ loop e = do
               traverse_ (displayI outputLoc) namesToDisplay
             ShowDefinitionI outputLoc showDefinitionScope query -> handleShowDefinition outputLoc showDefinitionScope query
             EditNamespaceI paths -> handleEditNamespace LatestFileLocation paths
-            FindShallowI pathArg -> do
-              Cli.Env {codebase} <- ask
-
-              pathArgAbs <- Cli.resolvePath' pathArg
-              entries <- liftIO (Backend.lsAtPath codebase Nothing pathArgAbs)
-              Cli.setNumberedArgs $ fmap (SA.ShallowListEntry pathArg) entries
-              pped <- Cli.currentPrettyPrintEnvDecl
-              let suffixifiedPPE = PPED.suffixifiedPPE pped
-              -- This used to be a delayed action which only forced the loading of the root
-              -- branch when it was necessary for printing the results, but that got wiped out
-              -- when we ported to the new Cli monad.
-              -- It would be nice to restore it, but it's pretty rare that it actually results
-              -- in an improvement, so perhaps it's not worth the effort.
-              let buildPPE = pure suffixifiedPPE
-              Cli.respond $ ListShallow buildPPE entries
+            FindShallowI pathArg -> handleLs pathArg
             FindI isVerbose fscope ws -> handleFindI isVerbose fscope ws input
             StructuredFindI _fscope ws -> handleStructuredFindI ws
             StructuredFindReplaceI ws -> handleStructuredFindReplaceI ws
@@ -1564,7 +1551,7 @@ checkDeletes typesTermsTuples doutput inputs = do
         (Path.HQSplit', Set Reference, Set Referent) ->
         Cli (Path.Split, Name, Set Reference, Set Referent)
       toSplitName hq = do
-        resolvedPath <- Path.convert <$> Cli.resolveSplit' (HQ'.toName <$> hq ^. _1)
+        resolvedPath <- first Path.unabsolute <$> Cli.resolveSplit' (HQ'.toName <$> hq ^. _1)
         return (resolvedPath, Path.unsafeToName (Path.unsplit resolvedPath), hq ^. _2, hq ^. _3)
   -- get the splits and names with terms and types
   splitsNames <- traverse toSplitName typesTermsTuples
@@ -1711,7 +1698,7 @@ docsI src = do
        (codebaseByName) Lastly check for `foo.doc` in the codebase and if found do `display foo.doc`
     -}
     dotDoc :: HQ.HashQualified Name
-    dotDoc = Name.convert . Name.joinDot src $ Name.fromSegment NameSegment.docSegment
+    dotDoc = HQ.NameOnly . Name.joinDot src $ Name.fromSegment NameSegment.docSegment
 
     findInScratchfileByName :: Cli ()
     findInScratchfileByName = do