highlight stray parenthesis, braces, brackets, quotes, and commas in red

[cdepillabout]

pretty-simple currently parses parentheses, braces, brackets, etc to neatly display nested data structures.

It would be nice to have stray parentheses, braces, brackets, etc be highlighted in red so the user could tell they are strange.

For instance, imagine a Haskell data type that has a show instance that looks like the following:

Foo { foo1 = 3.3, foo2 = "hello" } }

It would be nice if pretty-simple highlighted the last } in red so the user could see that something is strange.

[andrew-lei]

What is supposed to happen when the close bracket type does not match the open? This is what I wrote for ExprParser.hs:

parseCSep end s@(c:cs)
  | c == end = ([], cs)
  -- Mismatch condition; if the end does not match, there is a mistake
  -- Perhaps there should be a Missing constructor for Expr
  | c `elem` (")]}" :: String) = ([], s)

Basically, it will keep dropping out of brackets until it reaches a bracket set where the end type is correct.

I think perhaps the missing brackets could be inserted and highlighted in red as well.

[cdepillabout]

In general I would like pretty-simple to change the input as little as possible. (Well, aside from formatting!)

So ideally it wouldn't drop (or not print) anything that is in the input. If it comes across a bracket it is not expecting, I think it should print it out and highlight it in red.

I am worried that users would lose confidence in using pretty-simple if it drops some things.

For example, if you imagine an input as follows with an extra bracket on the end:

Foo { bar = 3, baz = "hello" } }

I think that it should be printed like the following:

Foo
    { bar = 3
    , baz = "hello"
    }
    }

Or maybe like this:

Foo
    { bar = 3
    , baz = "hello"
    }
}

The last } should be highlighted in red. We should go with the one that is easier to implement.

What is supposed to happen when the close bracket type does not match the open? This is what I wrote for ExprParser.hs

In this case, I think the unmatched bracket type should still be print out, but it should be print out in red.

For instance, imagine the following input:

Foo { bar = 3, baz = "hello" ] }

I think this should be formatted the following way:

Foo
    { bar = 3
    , baz = "hello" ]
    }

Or maybe something like:

Foo
    { bar = 3
    , baz = "hello"
    ]
    }

Again, whichever is easier to implement.

The last ] should be highlighted in red.

Here's another example:

Foo { bar = 3, baz = "hello" [ }

To be honest, I'm not exactly sure how this should look, but I would imagine it to be something like the following:

Foo
    { bar = 3
    , baz = "hello"
        [ }

The last } will be printed out in red.

Again, I'm not super concerned with the formatting of these rare cases.

It would also be great to get some doctests showing these strange cases.

[andrew-lei]

To clarify, when I said it 'drops out' of brackets, I meant the bracket layer, the Expr has reached the end at that point. Input is not dropped. I think it probably does insert an extra end bracket because the expression has ended before it sees the matching end bracket, but it currently will not drop anything.

Anyway, in that case, parseCSep should only check for the matching end bracket. Or perhaps still check for that, but add an Other with the non-matching end bracket to the list.

[cdepillabout]

@andrew-lei Thanks for the clarification.

I actually just tried running pString on the input from above, and it looks like the last bracket is droped:

> Data.Text.Lazy.IO.putStrLn $ pString "Foo { bar = 3, baz = \"hello\" } }"
Foo 
    { bar = 3
    , baz = "hello" 
    }

I also tried the other cases I gave above just to see what would happen:

> Data.Text.Lazy.IO.putStrLn $ pString "Foo { bar = 3, baz = \"hello\" ] }"
Foo 
    { bar = 3
    , baz = "hello" 
    }

> Data.Text.Lazy.IO.putStrLn $ pString "Foo { bar = 3, baz = \"hello\" [ }"
Foo 
    { bar = 3
    , baz = "hello" [ ]
    } 

Anyway, in that case, parseCSep should only check for the matching end bracket. Or perhaps still check for that, but add an Other with the non-matching end bracket to the list.

Yeah, this sounds good to me! Adding some doctests for the cases above would make it easy to make sure pretty-simple isn't dropping or adding any brackets/braces/parens.

[andrew-lei]

That is probably because I have parseExprs ending on one of ")]},"; parseExprs is used both to recursively to get [Expr] that are within brackets and to parse the entire string, and the former use requires knowledge of when to terminate.

Probably can fix this by changing signature from String -> ([Expr], String) to String -> String -> ([Expr], String); then, the first string denotes termination characters, and for its usage in expressionParse it will take an empty string for the first parameter.

Actually, now that I think about it, seems like that pattern could be used for all of these. The only issue might be that some of them keep the termination character in the continuation string, and some of them drop it, but I'll see if anything works.

Incidentally, the explanation for the first is that parseExprs exits on the end brace. On the second, upon encountering ], the parser drops to the next level of brackets, but then it's on the top level and so parseExprs encounters it as the end. On the third one, it encounters the end brace before seeing any end square bracket, so it drops out of the square brackets to the next level, which are the braces, and then applies that last brace.

The current behaviour is that anything after an extra end bracket at the top level is dropped, so my previous comment was incorrect (I was rather thinking of extra brackets on the inner levels).

[cdepillabout]

Actually, now that I think about it, seems like that pattern could be used for all of these. The only issue might be that some of them keep the termination character in the continuation string, and some of them drop it, but I'll see if anything works.

Yeah, this sounds like a good plan!

[andrew-lei]

In order to prevent extra brackets from being inserted at the end, as will happen when the parser runs out of input, the Exprs for the brackets will need an additional parameter for determining whether or not it was closed. Unless you have another idea of how to handle it?

[cdepillabout]

I think adding a parameter for determining whether or not it was closed sounds like a good idea. Is there some reason you're hesitant to do this?

[andrew-lei]

No reason, I just wanted to check.