diff --git a/unison-cli/src/Unison/Codebase/Editor/HandleInput/Merge2.hs b/unison-cli/src/Unison/Codebase/Editor/HandleInput/Merge2.hs index d4e75952e..a8cfb7183 100644 --- a/unison-cli/src/Unison/Codebase/Editor/HandleInput/Merge2.hs +++ b/unison-cli/src/Unison/Codebase/Editor/HandleInput/Merge2.hs @@ -259,8 +259,8 @@ doMerge info = do declNameLookup <- Cli.runTransaction (checkDeclCoherency db.loadDeclNumConstructors defns) & onLeftM \err -> Cli.returnEarly case err of - IncoherentDeclReason'ConstructorAlias name1 name2 -> - Output.MergeConstructorAlias who name1 name2 + IncoherentDeclReason'ConstructorAlias typeName conName1 conName2 -> + Output.MergeConstructorAlias who typeName conName1 conName2 IncoherentDeclReason'MissingConstructorName name -> Output.MergeMissingConstructorName who name IncoherentDeclReason'NestedDeclAlias shorterName longerName -> Output.MergeNestedDeclAlias who shorterName longerName diff --git a/unison-cli/src/Unison/Codebase/Editor/Output.hs b/unison-cli/src/Unison/Codebase/Editor/Output.hs index 02c188038..397813e83 100644 --- a/unison-cli/src/Unison/Codebase/Editor/Output.hs +++ b/unison-cli/src/Unison/Codebase/Editor/Output.hs @@ -399,7 +399,7 @@ data Output | MergeConflictedTermName !Name !(NESet Referent) | MergeConflictedTypeName !Name !(NESet TypeReference) | MergeConflictInvolvingBuiltin !Name - | MergeConstructorAlias !MergeSourceOrTarget !Name !Name + | MergeConstructorAlias !MergeSourceOrTarget !Name !Name !Name | MergeDefnsInLib !MergeSourceOrTarget | MergeMissingConstructorName !MergeSourceOrTarget !Name | MergeNestedDeclAlias !MergeSourceOrTarget !Name !Name diff --git a/unison-cli/src/Unison/CommandLine/OutputMessages.hs b/unison-cli/src/Unison/CommandLine/OutputMessages.hs index 0dabe5c9e..e4465b1a3 100644 --- a/unison-cli/src/Unison/CommandLine/OutputMessages.hs +++ b/unison-cli/src/Unison/CommandLine/OutputMessages.hs @@ -1341,13 +1341,43 @@ notifyUser dir = \case <> "was already up-to-date with" <> P.group (prettyMergeSource aliceAndBob.bob <> ".") MergeConflictedAliases aliceOrBob name1 name2 -> - pure . P.wrap $ - "On" - <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") - <> prettyName name1 - <> "and" - <> prettyName name2 - <> "are not aliases, but they used to be." + pure $ + P.wrap "Sorry, I wasn't able to perform the merge:" + <> P.newline + <> P.newline + <> P.wrap + ( "On the merge ancestor," + <> prettyName name1 + <> "and" + <> prettyName name2 + <> "were aliases for the same definition, but on" + <> prettyMergeSourceOrTarget aliceOrBob + <> "the names have different definitions currently. I'd need just a single new definition to use in their" + <> "dependents when I merge." + ) + <> P.newline + <> P.newline + <> P.wrap ("Please fix up" <> prettyMergeSourceOrTarget aliceOrBob <> "to resolve this. For example,") + <> P.newline + <> P.newline + <> P.indentN + 2 + ( P.bulleted + [ P.wrap + ( IP.makeExample' IP.update + <> "the definitions to be the same again, so that there's nothing for me to decide." + ), + P.wrap + ( IP.makeExample' IP.moveAll + <> "or" + <> IP.makeExample' IP.delete + <> "all but one of the definitions; I'll use the remaining name when propagating updates." + ) + ] + ) + <> P.newline + <> P.newline + <> P.wrap "and then try merging again." MergeConflictedTermName name _refs -> pure . P.wrap $ "The term name" <> prettyName name <> "is ambiguous. Please resolve the ambiguity before merging." @@ -1355,31 +1385,66 @@ notifyUser dir = \case pure . P.wrap $ "The type name" <> prettyName name <> "is ambiguous. Please resolve the ambiguity before merging." MergeConflictInvolvingBuiltin name -> - pure . P.wrap $ - "There's a merge conflict on" - <> P.group (prettyName name <> ",") - <> "but it's a builtin on one or both branches. We can't yet handle merge conflicts on builtins." - MergeConstructorAlias aliceOrBob name1 name2 -> - pure . P.wrap $ - "On" - <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") - <> prettyName name1 - <> "and" - <> prettyName name2 - <> "are aliases. Every type declaration must have exactly one name for each constructor." + pure . P.lines $ + [ P.wrap "Sorry, I wasn't able to perform the merge:", + "", + P.wrap + ( "There's a merge conflict on" + <> P.group (prettyName name <> ",") + <> "but it's a builtin on one or both branches. I can't yet handle merge conflicts involving builtins." + ), + "", + P.wrap + ( "Please eliminate this conflict by updating one branch or the other, making" + <> prettyName name + <> "the same on both branches, or making neither of them a builtin, and then try the merge again." + ) + ] + MergeConstructorAlias aliceOrBob typeName conName1 conName2 -> + pure . P.lines $ + [ P.wrap "Sorry, I wasn't able to perform the merge:", + "", + P.wrap $ + "On" + <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") + <> "the type" + <> prettyName typeName + <> "has a constructor with multiple names, and I can't perform a merge in this situation:", + "", + P.indentN 2 (P.bulleted [prettyName conName1, prettyName conName2]), + "", + P.wrap "Please delete all but one name for each constructor, and then try merging again." + ] MergeDefnsInLib aliceOrBob -> - pure . P.wrap $ - "On" - <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") - <> "there's a type or term directly in the `lib` namespace, but I expected only library dependencies to be in there." - <> "Please remove it before merging." + pure . P.lines $ + [ P.wrap "Sorry, I wasn't able to perform the merge:", + "", + P.wrap $ + "On" + <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") + <> "there's a type or term at the top level of the `lib` namespace, where I only expect to find" + <> "subnamespaces representing library dependencies.", + "", + P.wrap "Please move or remove it and then try merging again." + ] MergeMissingConstructorName aliceOrBob name -> - pure . P.wrap $ - "On" - <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") - <> "the type" - <> prettyName name - <> "is missing a name for one of its constructors. Please add one before merging." + pure . P.lines $ + [ P.wrap "Sorry, I wasn't able to perform the merge:", + "", + P.wrap $ + "On" + <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") + <> "the type" + <> prettyName name + <> "has some constructors with missing names, and I can't perform a merge in this situation.", + "", + P.wrap $ + "You can use" + <> IP.makeExample IP.view [prettyName name] + <> "and" + <> IP.makeExample IP.aliasTerm ["", prettyName name <> "."] + <> "to give names to each unnamed constructor, and then try the merge again." + ] MergeNestedDeclAlias aliceOrBob shorterName longerName -> pure . P.wrap $ "On" @@ -1388,15 +1453,25 @@ notifyUser dir = \case <> prettyName longerName <> "is an alias of" <> P.group (prettyName shorterName <> ".") - <> "Type aliases cannot be nested. Please make them disjoint before merging." + <> "I'm not able to perform a merge when a type exists nested under an alias of itself. Please separate them or" + <> "delete one copy, and then try merging again." MergeStrayConstructor aliceOrBob name -> - pure . P.wrap $ - "On" - <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") - <> "the constructor" - <> prettyName name - <> "is not in a subnamespace of a name of its type." - <> "Please either delete it or rename it before merging." + pure . P.lines $ + [ P.wrap $ + "Sorry, I wasn't able to perform the merge, because I need all constructor names to be nested somewhere" + <> "beneath the corresponding type name.", + "", + P.wrap $ + "On" + <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",") + <> "the constructor" + <> prettyName name + <> "is not nested beneath the corresponding type name. Please either use" + <> IP.makeExample' IP.moveAll + <> "to move it, or if it's an extra copy, you can simply" + <> IP.makeExample' IP.delete + <> "it. Then try the merge again." + ] PreviewMergeAlreadyUpToDate src dest -> pure . P.callout "😶" $ P.wrap $ diff --git a/unison-merge/src/Unison/Merge/DeclCoherencyCheck.hs b/unison-merge/src/Unison/Merge/DeclCoherencyCheck.hs index b62b9f44d..b2780772d 100644 --- a/unison-merge/src/Unison/Merge/DeclCoherencyCheck.hs +++ b/unison-merge/src/Unison/Merge/DeclCoherencyCheck.hs @@ -120,7 +120,7 @@ data IncoherentDeclReason -- Foo#Foo -- Foo.Bar#Foo#0 -- Foo.Some.Other.Name.For.Bar#Foo#0 - IncoherentDeclReason'ConstructorAlias !Name !Name + IncoherentDeclReason'ConstructorAlias !Name !Name !Name -- type, first constructor, second constructor | IncoherentDeclReason'MissingConstructorName !Name | -- | A second naming of a decl was discovered underneath its name, e.g. -- @@ -161,7 +161,7 @@ checkDeclCoherency loadDeclNumConstructors = Nothing -> Left (IncoherentDeclReason'StrayConstructor name1) Just (typeName, expected) -> case recordConstructorName conId name1 expected of - Left existingName -> Left (IncoherentDeclReason'ConstructorAlias existingName name1) + Left existingName -> Left (IncoherentDeclReason'ConstructorAlias typeName existingName name1) Right expected1 -> Right (typeName, expected1) where name1 = fullName name diff --git a/unison-src/transcripts/merge.output.md b/unison-src/transcripts/merge.output.md index ba3ab0d03..90412693d 100644 --- a/unison-src/transcripts/merge.output.md +++ b/unison-src/transcripts/merge.output.md @@ -964,8 +964,21 @@ baz = "baz" ```ucm project/alice> merge /bob - On project/alice, bar and foo are not aliases, but they used - to be. + Sorry, I wasn't able to perform the merge: + + On the merge ancestor, bar and foo were aliases for the same + definition, but on project/alice the names have different + definitions currently. I'd need just a single new definition + to use in their dependents when I merge. + + Please fix up project/alice to resolve this. For example, + + * `update` the definitions to be the same again, so that + there's nothing for me to decide. + * `move` or `delete` all but one of the definitions; I'll + use the remaining name when propagating updates. + + and then try merging again. ``` ### Conflict involving builtin @@ -990,9 +1003,15 @@ unique type MyNat = MyNat Nat ```ucm project/alice> merge /bob + Sorry, I wasn't able to perform the merge: + There's a merge conflict on MyNat, but it's a builtin on one - or both branches. We can't yet handle merge conflicts on + or both branches. I can't yet handle merge conflicts involving builtins. + + Please eliminate this conflict by updating one branch or the + other, making MyNat the same on both branches, or making + neither of them a builtin, and then try the merge again. ``` ### Constructor alias @@ -1019,9 +1038,16 @@ bob = 100 ```ucm project/alice> merge /bob - On project/alice, Foo.Bar and Foo.some.other.Alias are - aliases. Every type declaration must have exactly one name for - each constructor. + Sorry, I wasn't able to perform the merge: + + On project/alice, the type Foo has a constructor with multiple + names, and I can't perform a merge in this situation: + + * Foo.Bar + * Foo.some.other.Alias + + Please delete all but one name for each constructor, and then + try merging again. ``` ### Missing constructor name @@ -1048,8 +1074,14 @@ bob = 100 ```ucm project/alice> merge /bob - On project/alice, the type Foo is missing a name for one of - its constructors. Please add one before merging. + Sorry, I wasn't able to perform the merge: + + On project/alice, the type Foo has some constructors with + missing names, and I can't perform a merge in this situation. + + You can use `view Foo` and + `alias.term Foo.` to give names to + each unnamed constructor, and then try the merge again. ``` ### Nested decl alias @@ -1081,9 +1113,10 @@ bob = 100 ```ucm project/alice> merge /bob - On project/alice, the type A.inner.X is an alias of A. Type - aliases cannot be nested. Please make them disjoint before - merging. + On project/alice, the type A.inner.X is an alias of A. I'm not + able to perform a merge when a type exists nested under an + alias of itself. Please separate them or delete one copy, and + then try merging again. ``` ### Stray constructor alias @@ -1115,9 +1148,14 @@ project/bob> add ```ucm project/alice> merge bob + Sorry, I wasn't able to perform the merge, because I need all + constructor names to be nested somewhere beneath the + corresponding type name. + On project/alice, the constructor AliasOutsideFooNamespace is - not in a subnamespace of a name of its type. Please either - delete it or rename it before merging. + not nested beneath the corresponding type name. Please either + use `move` to move it, or if it's an extra copy, you can + simply `delete` it. Then try the merge again. ``` ### Term or type in `lib` @@ -1139,9 +1177,13 @@ bob = 100 ```ucm project/alice> merge /bob - On project/alice, there's a type or term directly in the `lib` - namespace, but I expected only library dependencies to be in - there. Please remove it before merging. + Sorry, I wasn't able to perform the merge: + + On project/alice, there's a type or term at the top level of + the `lib` namespace, where I only expect to find subnamespaces + representing library dependencies. + + Please move or remove it and then try merging again. ``` ## LCA precondition violations