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.

Sign up for GitHub

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

Jump to bottom

Fix non-idempotent comment formatting #2101

Merged
merged 1 commit into from
Nov 21, 2020
Merged

Fix non-idempotent comment formatting #2101

merged 1 commit into from
Nov 21, 2020

Commits on Nov 21, 2020

  1. Fix non-idempotent comment formatting 

    The purpose of this change is to fix the following test failure:

```
    Formatting should be idempotent:                                                                                           FAIL (3.88s)
      *** Failed! Falsified (after 8396 tests and 12 shrinks):
      ASCII
      Header ""
      RecordCompletion (RecordCompletion (BoolLit False) (ImportAlt TextShow (Field (Pi "a" (Field (IntegerLit 0) (FieldSelection {fieldSelectionSrc0 = Nothing, fieldSelectionLabel = "", fieldSelectionSrc1 = Nothing})) Double) (FieldSelection {fieldSelectionSrc0 = Nothing, fieldSelectionLabel = "1aaa", fieldSelectionSrc1 = Nothing})))) (Let (Binding {bindingSrc0 = Just (Src {srcStart = SourcePos {sourceName = "", sourceLine = Pos 17, sourceColumn = Pos 8}, srcEnd = SourcePos {sourceName = "", sourceLine = Pos 6, sourceColumn = Pos 4}, srcText = "--\"zTep.;Bkr \n\n"}), variable = "", bindingSrc1 = Nothing, annotation = Nothing, bindingSrc2 = Nothing, value = NaturalOdd}) (Const Type))
      "( False::(Text/show ? (forall (a : +0.``) -> Double).`1aaa`)\n)::( let --\"zTep.;Bkr\n\n         `` =\n           Natural/odd\n\n     in  Type\n   )\n" /= "(False::(Text/show ? (forall (a : +0.``) -> Double).`1aaa`))::( let --\"zTep.;Bkr\n\n                                                                    `` =\n                                                                      Natural/odd\n\n                                                                in  Type\n                                                              )\n"
      Use --quickcheck-replay=344421 to reproduce.
```

What's happening is that the test attempts to format this expression

```
(False::(Text/show ? (forall (a : +0.``) -> Double).`1aaa`))::( let --"zTep.;Bkr
                                                                    `` =                                                                              Natural/odd

                                                                in  Type                                                                      )
```

… where there is a space right after the `;Bkr` at column 81.  The formatter
takes that trailing space into account when formatting the expression,
producing this result:

```
( False::(Text/show ? (forall (a : +0.``) -> Double).`1aaa`)
)::( let --"zTep.;Bkr

         `` =
           Natural/odd

     in  Type
   )
```

… but the formatted result strips the trailing space from the comment *after
formatting* (because of the use of `Pretty.removeTrailingWhitespace`).  Then
the second attempt to format the expression goes back to the first
form since the space is gone and now the original expression fits in 80
columns.

The way I worked around this was to strip trailing whitespace from the
comment so that the formatting algorithm would correctly take that into
account while formatting.
    @Gabriella439
    Gabriella439 committed Nov 21, 2020
    Configuration menu
    Copy the full SHA
    464b31d View commit details
    Browse the repository at this point in the history