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

Diff list values #450

Merged
merged 5 commits into from
Jun 10, 2018
Merged

Conversation

basile-henry
Copy link
Collaborator

Issue #431

Make the diff of two lists actually show the diff of the values inside.

For example with example1.dhall:

{ a = [1, 2, 3, 4, 5]
, b = [1, 2, 3, 4, 5] : List Natural
, c = [1, 2, 3, 4, 5] : List Natural
, d = [] : List Natural
, e = [] : List Natural
, f = [1, 2, 3, 4, 5] : List Natural
}

And example2.dhall:

{ a = [1, 6, 7, 4, 5]
, b = [1, 2, 3] : List Natural
, c = [2, 3, 4, 5]
, d = [] : List Natural
, e = [] : List Integer
, f = [+1, +2, +3, +4, +5] : List Integer
}

The diff looks as follows:

➜ dhall diff "./example1.dhall" "./example2.dhall"
{ a = [ …
      , - 2
      , - 3
      , + 6
      , + 7
      , …
      ]
       
, b = [ …
      , - 4
      , - 5
      ]
       
, c = [ - 1
      , …
      ]
       
, d = - [ … ]
      + [ … ]
      : …
        … 
, e = - [ … ]
      + [ … ]
      : …
        - Natural
        + Integer
      
, f = - [ … ]
      + [ … ]
       
}

Issue dhall-lang#431

Make the diff of two lists actually show the diff of the values inside.

For example with `example1.dhall`:

```dhall
{ a = [1, 2, 3, 4, 5]
, b = [1, 2, 3, 4, 5] : List Natural
, c = [1, 2, 3, 4, 5] : List Natural
, d = [] : List Natural
, e = [] : List Natural
, f = [1, 2, 3, 4, 5] : List Natural
}
```

And `example2.dhall`:

```dhall
{ a = [1, 6, 7, 4, 5]
, b = [1, 2, 3] : List Natural
, c = [2, 3, 4, 5]
, d = [] : List Natural
, e = [] : List Integer
, f = [+1, +2, +3, +4, +5] : List Integer
}
```

The diff looks as follows:

```sh
➜ dhall diff "./example1.dhall" "./example2.dhall"
{ a = [ …
      , - 2
      , - 3
      , + 6
      , + 7
      , …
      ]

, b = [ …
      , - 4
      , - 5
      ]

, c = [ - 1
      , …
      ]

, d = - [ … ]
      + [ … ]
      : …
        …
, e = - [ … ]
      + [ … ]
      : …
        - Natural
        + Integer

, f = - [ … ]
      + [ … ]

}
```

Signed-off-by: Basile Henry <[email protected]>
Copy link
Collaborator

@Gabriella439 Gabriella439 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Just one minor suggestion

Also, I added you as a collaborator so that you can merge your pull request yourself

| otherwise = False

-- Render a single element of a list using an extra rendering function
prettyElems f = foldMap (pure . f . token . Internal.prettyExpr)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can do prettyElems f = map (f . token . Internal.prettyExpr) here

@basile-henry basile-henry merged commit bfb6152 into dhall-lang:master Jun 10, 2018
@basile-henry basile-henry deleted the dhall-diff-lists branch June 10, 2018 09:55
Gabriella439 added a commit that referenced this pull request Aug 7, 2019
Fixes #1211

The original implementation of diffing two lists in #450 attempted to add a
special case in the logic for when the list had no common elements.
However, this led to a bug where the two lists would display different
if they were both empty.

lists would never render.

This change fixes that by removing the special case.

The new output for the linked example is:

```
Use "dhall --explain" for detailed errors

Error: Assertion failed

[ …
, - 1
, …
, + 55
]

16│                assert : fibs 10 ≡ [ 0, 1, 2, 3, 5, 8, 13, 21, 34, 55 ]
17│

(stdin):16:16
```
Gabriella439 added a commit that referenced this pull request Aug 7, 2019
Fixes #1211

The original implementation of diffing two lists in #450 attempted to add a
special case in the logic for when the list had no common elements.
However, this led to a bug where the two lists would display different
if they were both empty.

lists would never render.

This change fixes that by removing the special case.

The new output for the linked example is:

```
Use "dhall --explain" for detailed errors

Error: Assertion failed

[ …
, - 1
, …
, + 55
]

16│                assert : fibs 10 ≡ [ 0, 1, 2, 3, 5, 8, 13, 21, 34, 55 ]
17│

(stdin):16:16
```
@Gabriella439 Gabriella439 mentioned this pull request Aug 7, 2019
mergify bot pushed a commit that referenced this pull request Aug 8, 2019
Fixes #1211

The original implementation of diffing two lists in #450 attempted to add a
special case in the logic for when the list had no common elements.
However, this led to a bug where the two lists would display different
if they were both empty.

lists would never render.

This change fixes that by removing the special case.

The new output for the linked example is:

```
Use "dhall --explain" for detailed errors

Error: Assertion failed

[ …
, - 1
, …
, + 55
]

16│                assert : fibs 10 ≡ [ 0, 1, 2, 3, 5, 8, 13, 21, 34, 55 ]
17│

(stdin):16:16
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants