unisonweb/unison
1
0
Fork 0
mirror of https://github.com/unisonweb/unison.git synced 2024-08-15 21:40:50 +03:00
Code Issues Projects Releases Wiki Activity

Merge pull request #5032 from unisonweb/24-05-31-merge-precondition-messages

Browse Source 
improve merge precondition violation output messages
This commit is contained in:
mergify[bot] 2024-05-31 18:57:58 +00:00 committed by GitHub
commit 4c2fdbd2ff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 176 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
unison-cli/src/Unison/Codebase/Editor/HandleInput/Merge2.hs
View File

@ -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

2
unison-cli/src/Unison/Codebase/Editor/Output.hs
View File

@ -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

151
unison-cli/src/Unison/CommandLine/OutputMessages.hs
View File

@ -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 ["<hash>", prettyName name <> ".<ConstructorName>"]
<> "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 $

4
unison-merge/src/Unison/Merge/DeclCoherencyCheck.hs
View File

@ -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

74
unison-src/transcripts/merge.output.md
View File

@ -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 <hash> Foo.<ConstructorName>` 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