Add diff option for eval plugin

[Ailrun]

This PR adds an option to disable the diff behaviour of the eval plugin. Current eval plugin generates output like

-- >>> testParser pType "()"
-- WAS TyUnit
-- NOW TypUnit

when "refreshed". This itself is fine, I think. However, this becomes quickly unwieldy as it recursively generates

-- >>> testParser pType "()"
-- WAS WAS TyUnit
-- WAS NOW TypUnit
-- NOW TypUnit

and so on.

This PR provides an option to disable this diff entirely, so that if users (like me) want to disable it completely, they do.

[berberman]

LGTM. It would be better to add a test for this feature.

[pepeiborra]

I would prefer a feature fix for the plugin behaviour.

Is it possible to identify and delete old -- WAS... lines?

• If yes, let's do it!
• If no, is this diff feature still worth it?

[Ailrun]

@pepeiborra, Actually, I am personally more inclined to remove this feature, but just not sure whether there is a user who "uses" this feature. @tittoassini may have some opinion?

[pepeiborra]

@pepeiborra, Actually, I am personally more inclined to remove this feature, but just not sure whether there is a user who "uses" this feature. @tittoassini may have some opinion?

Is this because the feature is not useful, or because it gets in the way due to the current behaviour?

In other words, if the feature didn't duplicate WAS lines, would you find it useful?

[Ailrun]

Is this because the feature is not useful, or because it gets in the way due to the current behaviour?

For me, even if "WAS" duplication was not there, it's more like an OK behavior than good. In other words, it is at least personally not useful. It feels like leaving dynamic information obtained during development as static comments.

[tittoassini]

There is already a mechanism to disable the diff, just use a plain comment {- rather than a doc comment {- |

The reason for this difference in behaviour is that while you develop a function you expect the results of the function you are writing to change and might not bother about the diff (though removing all WAS/NOW is as simple as CTRL-Z).

But as soon as the code is finished the test results should be 'frozen' or at least it should be obvious when a change in the function implementation changes the test results as the tests are now part of the function definition.

A test that doesn't check its result is fairly useless.

[Ailrun]

There is already a mechanism to disable the diff, just use a plain comment {- rather than a doc comment {- |

Unfortunately, that mechanism doesn't help me much when I use both doctest and eval (I need to remove and add | repeatedly, and it is easy to forget to add it back, which are far from good DX IMO)

A test that doesn't check its result is fairly useless.

so yes, if you think it is an useful (or even important) feature of eval plugin, I think this PR is a good compromise, i.e., introducing an option to turn it off completely.

[tittoassini]

I have nothing against in additional option but yes, I would say that is quite important that a test actually checks its output and that this should be the default behaviour.

So you are running doctest in the background and at the same time you are using eval?

How do the two interfere with each other?

[Ailrun]

This file is completely altered because of \r (it looks like #2597 introducing some \rs. C.C. @bradrn).

[bradrn]

Oh, that’s really bad. The pre-commit hook should not change semantics. This is a stylish-haskell bug, I think.

[Ailrun]

@bradrn It didn't change the semantics. It just added \r on each line, which gives this "huge" git diff. (As my setting auto-removes \rs)

[bradrn]

Thanks for clarifying — I had thought you meant something different by ‘completely altered’.

I’m a bit confused, though. What exactly changed the file? My unrelated PR (merged after this one was submitted) shouldn’t affect this code, unless you merge it… in which case it shouldn’t show up in this commit. Did you run the pre-commit hook by any chance?

[Ailrun]

@bradrn I always rebase my PR even after its submission, so this PR is indeed after your PR. (the last was 8 hours ago: #2622 (comment))

[bradrn]

OK, makes sense, thanks!

[Ailrun]

So you are running doctest in the background and at the same time you are using eval?

How do the two interfere with each other?

Other than IO stuff, they work fine IMO.

[jneira]

Ok, i also for reduce config options as much as possible but it seems it is needed here, thoughts @pepeiborra?

[anka-213]

For me, using git seems like a strictly better way of getting a diff. If you've added the old version to the git index, you will see a diff in your git-aware editor, if not, you don't. No extra effort or config from hls needed.

[Ailrun]

@anka-213 TBH, I completely agree with you.

[Ailrun]

If there is no other comments, let me merge this PR.