Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix diffs involving unit/tuples #5462

Merged
merged 4 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions unison-share-api/src/Unison/Server/Backend/DefinitionDiff.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,18 @@ diffSyntaxText (AnnotatedText fromST) (AnnotatedText toST) =
-- treat these special cases as equal, then we can detect and expand them in a post-processing step.
diffEq :: AT.Segment Syntax.Element -> AT.Segment Syntax.Element -> Bool
diffEq (AT.Segment {segment = fromSegment, annotation = fromAnnotation}) (AT.Segment {segment = toSegment, annotation = toAnnotation}) =
fromSegment == toSegment ||
case (fromAnnotation, toAnnotation) of
fromSegment == toSegment
|| case (fromAnnotation, toAnnotation) of
(Nothing, _) -> False
(_, Nothing) -> False
(Just a, Just b) ->
case a of
-- The set of annotations we want to special-case
Syntax.TypeReference{} -> a == b
Syntax.TermReference{} -> a == b
Syntax.DataConstructorReference{} -> a == b
Syntax.AbilityConstructorReference{} -> a == b
Syntax.HashQualifier{} -> a == b
Syntax.TypeReference {} -> a == b
Syntax.TermReference {} -> a == b
Syntax.DataConstructorReference {} -> a == b
Syntax.AbilityConstructorReference {} -> a == b
Syntax.HashQualifier {} -> a == b
_ -> False

expandSpecialCases :: [Diff.Diff [AT.Segment (Syntax.Element)]] -> [SemanticSyntaxDiff]
Expand All @@ -67,18 +67,23 @@ diffSyntaxText (AnnotatedText fromST) (AnnotatedText toST) =
( \next acc -> case (acc, next) of
(Both xs : rest, Left seg) -> Both (seg : xs) : rest
(_, Left seg) -> Both [seg] : acc
(_, Right diff) -> diff : acc
(_, Right diff) -> diff ++ acc
)
detectSpecialCase :: AT.Segment Syntax.Element -> AT.Segment Syntax.Element -> Either (AT.Segment Syntax.Element) SemanticSyntaxDiff
detectSpecialCase :: AT.Segment Syntax.Element -> AT.Segment Syntax.Element -> Either (AT.Segment Syntax.Element) [SemanticSyntaxDiff]
detectSpecialCase fromSegment toSegment
| fromSegment == toSegment = Left fromSegment
| AT.annotation fromSegment == AT.annotation toSegment = Right (SegmentChange (AT.segment fromSegment, AT.segment toSegment) (AT.annotation fromSegment))
| AT.annotation fromSegment == AT.annotation toSegment = Right [SegmentChange (AT.segment fromSegment, AT.segment toSegment) (AT.annotation fromSegment)]
-- We only emit an annotation change if it's a change in just the hash of the element (optionally the KIND of hash reference can change too).
| AT.segment fromSegment == AT.segment toSegment,
Just _fromHash <- AT.annotation fromSegment >>= elementHash,
Just _toHash <- AT.annotation toSegment >>= elementHash =
Right (AnnotationChange (AT.segment fromSegment) (AT.annotation fromSegment, AT.annotation toSegment))
| otherwise = error "diffSyntaxText: found Syntax Elements in 'both' which have nothing in common."
Right [AnnotationChange (AT.segment fromSegment) (AT.annotation fromSegment, AT.annotation toSegment)]
| otherwise =
-- the annotation changed, but it's not a recognized hash change.
-- This can happen in certain special cases, e.g. a paren changed from being a syntax element into being part
-- of a unit.
-- We just emit both as old/new segments.
Right [Old [fromSegment], New [toSegment]]
where
elementHash :: Syntax.Element -> Maybe Syntax.UnisonHash
elementHash = \case
Expand Down
13 changes: 13 additions & 0 deletions unison-src/transcripts/definition-diff-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ take n s =
else None
{ r } -> Some r
handle s() with h n

id x = x
unitCase = id (x -> 1)

```

``` ucm
Expand Down Expand Up @@ -53,6 +57,9 @@ take n s =
if n > 0
then handle s () with h (n - 1)
else None

id x = x
unitCase = id (x -> (1, ()))
```

``` ucm
Expand All @@ -71,6 +78,12 @@ More complex diff
GET /api/projects/diffs/diff/terms?oldBranchRef=main&newBranchRef=new&oldTerm=take&newTerm=take
```

Regression test for weird behavior w/r to unit and parens.

``` api
GET /api/projects/diffs/diff/terms?oldBranchRef=main&newBranchRef=new&oldTerm=unitCase&newTerm=unitCase
```


Diff types

Expand Down
Loading
Loading