-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
Fix multiline eval plugin padding #2910
Conversation
- add test for multiline show instance See haskell#2907
830b3ee
to
2a4b9d9
Compare
@@ -358,8 +358,9 @@ runTests EvalConfig{..} e@(_st, _) tests = do | |||
dbg "TEST RESULTS" rs | |||
|
|||
let checkedResult = testCheck eval_cfg_diff (section, test) rs | |||
let resultLines = concatMap T.lines checkedResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I am unsure why we get [Text]
that does not have the show
result split into lines.
We might want to consider splitting in the evals
function. 🤷♂️
-- M { | ||
-- l1="first line", | ||
-- l2="second line" | ||
-- } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only tests --
comments, but I figured {- -}
would be fine and covered by the other multiline tests.
@pepeiborra Could you please take a look? 🙂 |
I'm not the owner of the Eval plugin, even though I was the original contributor. @tittoassini rewrote it and I have not been keeping up. If no one else can review your PR, it will have to wait until I have spare time (probably the weekend). |
Thanks, @pepeiborra, I am aware but I don't know about anyone else familiar with the plugin except maybe @Ailrun? Either way, I am in no hurry to merge this 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am aware but I don't know about anyone else familiar with the plugin except maybe @Ailrun?
You can tag me, though I'm sometimes not super reactive.
Anyway, looks good to me.
* Test multiline eval results - add test for multiline show instance See haskell#2907 * Fix multiline eval padding See haskell#2907
* Test multiline eval results - add test for multiline show instance See haskell#2907 * Fix multiline eval padding See haskell#2907
concatMap lines
)