marking code in error messages with backticks - #2079

This looks good to me, but I feel like regular quote characters (single or double) would be more suitable than backticks, and would match some of our other errors.

Alright. I'll make the change tonight.

Are you going with double quotes or single quotes? I think single quotes could be a problem since they can also be part of an identifier.

Good point. Maybe backticks is best actually, as we can also have quoted labels for records and accessors.

Yes, I initially chose backticks because ' or " can be used in other contexts :)

Hmmm, can't we have backticks in the quoted code too, when functions as operators? I didn't have the code snippet in the output when I used the deprecated syntax for operator sections to try to get a good example, but I think there will always be a few cases where we will have wrong quotes. I think colored output might be the best solution, but we don't do that anywhere else yet, right?

Another solution is to put code in an indented block, but that could lead to long errors.

@soupi Sorry, but this will need rebasing now due to other changes that went in. I may have made your job easier now though, as most types of name go through one function in the error printing.

Coverage decreased (-0.01%) to 56.038% when pulling a72d4da on soupi:0.9 into b5af366 on purescript:0.9.

Thanks for rebasing, there's one thing that will need a little tweak though - we probably want to move uses of markCode inside printName, as now it will print things like Unknown value x or `Unknown `type operator (~>).

Ah, all right. I'll make the necessary changes :)

Coverage decreased (-0.01%) to 56.038% when pulling 94bdff6 on soupi:0.9 into b5af366 on purescript:0.9.

Does this work if something spans multiple lines?

it does something like:

Error found:
in module `Slides`
at /Users/suppi/code/purescript/purescript-slides/src/Slides.purs line 56, column 27 - line 59, column 3

  Could not match type

    Slides

  with type

    { pos :: Int
    , slides :: Array Slide
    }


while trying to match type `HalogenFP _0 Slides`
  with type `HalogenFP EventSource    `
               { pos :: Int
               , slides :: Array Slide
               }
while checking that expression `(bind (modify (\$1 ->  `
                                                 ...
                                              )
                                      )
                                )
                                (\__unused ->
                                   pure next
                                )
  has type `Free                        `
              (HalogenFP EventSource
                 { pos :: Int
                 , slides :: Array Slide
                 }
                 Move
                 g0
              )
              a1
in value declaration `ui`

where `g0` is a rigid type variable
        bound at line 44, column 1 - line 65, column 1
      `a1` is a rigid type variable
        bound at line 56, column 3 - line 59, column 3
      `_0` is an unknown type

See https://github.com/purescript/purescript/wiki/Error-Code-TypesDoNotUnify for more information,
or to contribute content related to this error.

`Free                        `

hconcat 😆

We have a combinator to put the final tick in the right place somewhere in Pretty.Common.

Boxes should now look like this:

tests/TestCompiler.hs:197:
  90) Failing examples 'SuggestComposition.purs' should fail with 'TypesDoNotUnify'
       Error found:
       in module `SuggestComposition`
       at /Users/suppi/code/haskell/purescript/examples/failing/SuggestComposition.purs line 7, column 5 - line 7, column 10

         Could not match type

           Record

         with type

           Function Int


       while trying to match type `{ g :: _0
                                   | _1
                                   }        `
         with type `_2 -> _3`
       while checking that expression `g`
         has type `{ g :: _0
                   | _1
                   }        `
       while checking type of property accessor `g.g`
       in value declaration `f`

       where `_2` is an unknown type
             `_3` is an unknown type
             `_0` is an unknown type
             `_1` is an unknown type

       See https://github.com/purescript/purescript/wiki/Error-Code-TypesDoNotUnify for more information,
       or to contribute content related to this error.

I have also renamed withBoxBackTicks to markCodeBox

I don't know about this. I do appreciate all the work here, and the appearance really shouldn't matter, but the backticks just look .. odd. I'd rather go with coloring the types in the output, but then we'd need to modify the boxes library to support things spanning multiple lines. Is it really so bad to just leave things unquoted?

I'm not sure I'm satisfied with how it looks either and I think colored types will look better as well. Being able to immediately focus on the types is nice. Perhaps we should close this for now and open an issue about colored types instead? Or maybe ask in the #2056

I think it's #1517.

I agree with @paf31, yeah. Looking back at the original issue, as far as I can tell, the only ones that have a risk of causing confusion are "unknown type X" and "unknown value X" (imagine eg X is Error, but I guess there probably are others). What about quoting just those?

Coverage decreased (-0.02%) to 55.91% when pulling 9ab198a on soupi:0.9 into 4852b5d on purescript:0.9.

I can do that. should I do that?

In that case, it's probably just printName that needs to use markCode. Or any time a name appears in the middle of a sentence perhaps, so there are a bunch of other cases that are fine too.

Yes please 😀

Sent from my iPhone

On May 16, 2016, at 9:57 PM, suppi notifications@github.com wrote:

I can do that. should I do that?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

how about putting ansi colors in the regular markCode code and when there's a flag to remove them, for example with --json-errors, filter them out?

We could, but I think it would be a bit cleaner to just not put them there in the first place.

I've only added the yellow color around code. the rest are from the haskell testing framework I think :)

Note that I've also taken the liberty to choose whether some places will report errors with color or not, but I'm not very familiar with the compiler so I might have made some mistakes there as well.

Coverage decreased (-0.1%) to 56.061% when pulling 5adbd6f on soupi:0.9 into 83be9a5 on purescript:0.9.

Purely aesthetic: make sure to test in terminal with a light theme. I recall yellow working at work though.

Hmm, good point. Perhaps an alternative is something like bold text, or underlined, or swapping the background and foreground colors. Or just picking a different color, if necessary.

Worst case, we can provide a --monochrome flag.

Perhaps the Vivid color intensity is a bit too much for a light theme, maybe Dull will look better.
We can also support flags to change the color, or --monochrome, or choose a different color. I'm open to suggestions :)

Coverage decreased (-0.2%) to 55.994% when pulling 6956c35 on soupi:0.9 into 83be9a5 on purescript:0.9.

Coverage decreased (-0.2%) to 55.994% when pulling 79d3460 on soupi:0.9 into 83be9a5 on purescript:0.9.

Coverage decreased (-0.2%) to 55.994% when pulling 7624017 on soupi:0.9 into 83be9a5 on purescript:0.9.

Thanks!

What about just dimming the color?

It should be dull yellow now.