Skip to content

Commit

Permalink
default colours to 0 if not set from config fixes #330
Browse files Browse the repository at this point in the history
  • Loading branch information
Anthony Lloyd committed Jun 30, 2019
1 parent 978135d commit 9a6e19c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 20 deletions.
32 changes: 16 additions & 16 deletions Expecto.Tests/Tests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ let tests =
test "different length, actual is shorter" {
Expect.equal "Test" "Test2" "Failing - string with different length"
} |> assertTestFailsWithMsgStartingDelay (fun () ->
let redStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let redStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
"Failing - string with different length. String actual was shorter than expected, at pos 4 for expected item '2'.\n"+
greenStart+"expected"+greenEnd+": Test"+greenStart+"2"+greenEnd+"\n"+
redStart+" actual"+redEnd+": Test"
Expand All @@ -89,21 +89,21 @@ let tests =
test "different length, actual is longer" {
Expect.equal "Test2" "Test" "Failing - string with different length"
} |> assertTestFailsWithMsgStartingDelay (fun () ->
let redStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let redStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
"Failing - string with different length. String actual was longer than expected, at pos 4 found item '2'.\n"+
greenStart+"expected"+greenEnd+": Test\n"+
redStart+" actual"+redEnd+": Test"+redStart+"2"+redEnd
)
test "fail - different content" {
Expect.equal "Test" "Tes2" "Failing - string with different content"
} |> assertTestFailsWithMsgStartingDelay (fun () ->
let redStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let redStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
"Failing - string with different content. String does not match at position 3. Expected char: '2', but got 't'.\n"+
greenStart+"expected"+greenEnd+": Tes"+greenStart+"2"+greenEnd + "\n"+
redStart+" actual"+redEnd+": Tes"+redStart+"t"+redEnd
Expand All @@ -118,10 +118,10 @@ let tests =
test "fail - different content" {
Expect.equal {a = "dd"; b = "de" } {a = "dd"; b = "dw" } "Failing - record with different content"
} |> assertTestFailsWithMsgStartingDelay (fun () ->
let redStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let redStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
"Failing - record with different content.\nRecord does not match at position 2 for field named `b`. Expected field with value: \"dw\", but got \"de\".\n"+
greenStart+"expected"+greenEnd+":\n{a = \"dd\";\n b = \"d"+greenStart+"w"+greenEnd+"\";}\n"+
redStart+" actual"+redEnd+":\n{a = \"dd\";\n b = \"d"+redStart+"e"+redEnd+"\";}"
Expand Down
8 changes: 4 additions & 4 deletions Expecto/Expect.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ let private allDiffs (s1:string) (s2:string) =
|> snd

let private highlightAllRed diffs (s:string) =
let redStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let redStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Red
let redEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
let l = s.Length
List.fold (fun (s:string) (i,j) ->
if i>=l then s
else s.Insert(min j l,redEnd).Insert(i,redStart)
) s diffs

let private highlightAllGreen diffs (s:string) =
let greenStart = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText ANSIOutputWriter.colours.Value ConsoleColor.Cyan
let greenStart = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Green
let greenEnd = ANSIOutputWriter.colourText (ANSIOutputWriter.getColour()) ConsoleColor.Cyan
let l = s.Length
List.fold (fun (s:string) (i,j) ->
if i>=l then s
Expand Down
1 change: 1 addition & 0 deletions Expecto/Logging.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ module internal ANSIOutputWriter =

let mutable internal colours = None

This comment has been minimized.

Sign in to view

Copy link
@haf

haf Jun 30, 2019

Owner

I don't particularly like mutable variables; they make the code-base hard to reason about. I introduced DVar which at least lets you react to these mutable variables changing, but preferrably, we have a default config.

This comment has been minimized.

Sign in to view

Copy link
@AnthonyLloyd

AnthonyLloyd Jul 1, 2019

Contributor

I found the DVar complex and didn't add anything. I couldn't correctly add a new variable to it. I think the whole logging could be dramatically simplified but haven't attempted this yet. You can DVar it and we can see the difference.
let internal setColourLevel c = if colours.IsNone then colours <- Some c
let internal getColour() = Option.defaultValue Colour0 colours

This comment has been minimized.

Sign in to view

Copy link
@haf

haf Jun 30, 2019

Owner

This is not a good default. Windows 7 was released a decade ago; if someone is having problems on it, they should upgrade or add the --colours flag at the very least.

This comment has been minimized.

Sign in to view

Copy link
@AnthonyLloyd

AnthonyLloyd Jul 1, 2019

Contributor

It's not for Win7.

let colourReset = "\u001b[0m"

Expand Down

2 comments on commit 9a6e19c

@haf
Copy link
Owner

@haf haf commented on 9a6e19c Jun 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I veto this change; I don't want 0 colours by default, please revert this commit.

@AnthonyLloyd
Copy link
Contributor

Choose a reason for hiding this comment

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

This is for #330 which is a use case where not running through runTests but just using Expect. I'm really sure of why anyone would want this (script??) and so put it at the simplest default.

Please sign in to comment.