Use diff engine

[SimonCropp]

• replaces "Fix difftools args" Fix difftools args #593
• fixes "ShouldMatchApproved opens multiple instances of difftool" ShouldMatchApproved opens multiple instances of difftool #388
• adds support for AraxisMerge, DeltaWalker, Diffinity, DiffMerge, ExamDiff, Guiffy, Kaleidoscope, Meld, Rider, SublimeMerge, TkDiff, Vim

@josephwoodward @jnm2 @slang25 thoughts on this one? i know it adds a dependency, but this is the purpose i build diffengine for. I have not added process cleanup yet. i figured i would check if the dependency was something that is acceptable first.

[jnm2]

How will existing code using Shouldly v3 know what to do upon upgrading to v4?

[jnm2]

Is it possible in principle to preserve Shouldly's existing API and delegate to DiffEngine for the implementation?

[SimonCropp]

How will existing code using Shouldly v3 know what to do upon upgrading to v4?

I added obsoletes

Is it possible in principle to preserve Shouldly's existing API and delegate to DiffEngine for the implementation

i would prefer not to. reasons:

• many of the diff APIs should not have been made public. eg KnownDoNotLaunchStrategies, DoNotLaunchWhenPlatformIsNotWindows
• there is a mismatch between the APIs. for example shouldly is hard coded to expect that a single tool has the same args for different OS. this is not correct eg https://github.com/VerifyTests/DiffEngine/blob/master/docs/diff-tool.md#windows-settings so that cant be properly represented in the existing shouldly api. The may be other examples i have not found, and future changes in DiffEngine could result in more of a mismatch
• most of the shouldly Diff api should be very rarely used required by people. for example given the tools supported by DiffEngine people should rarely need to add a custom tool
• shouldly lack opinion on if settings should be personal or shared
• the shouldly API docs would need be continually maintained

[SimonCropp]

I have toyed with the idea of avoiding a nuget ref. submodules or a source package. but ultimately the docs was too difficult to replicate

[SimonCropp]

thoughts on the above? this PR changes enough that i am holding of on the next few things

[jnm2]

That answers my question, thanks. I was thinking it could ease the transition, but if we want the obsoleted APIs to continue working in v4, we should just leave the v3 impl intact. If we don't, I think I'm good with that with the caveat that I haven't used diffing so I don't have a good sense of what people might expect.

[slang25]

I like this in principle, the overhead of properly wrapping diff tools on different platforms is quite significant. I'm convinced the issues that have been raised over the last couple of years around this is only the tip of the iceberg.

If we are bumping the major version then changing the API seems to make sense, can we cover off the same sort of configuration points? For example, being able to influence what diff tools to search for. How is the DiffEngine config exposed?

[SimonCropp]

@slang25

can we cover off the same sort of configuration points? For example, being able to influence what diff tools to search for. How is the DiffEngine config exposed?

I added obsoletes https://github.com/shouldly/shouldly/pull/644/files#diff-70e3573de61136ab637e3d710244ccdaR849

[SimonCropp]

@josephwoodward any more thoughts on this one?

[josephwoodward]

Are there any changes in behaviour users will/may see between v3 (the old way) and this? Initial ones that come to mind are:

• Will the filenames remain the same (the whatever.approved.cs file)?
• Changes in the contents of the approved file?

[SimonCropp]

Will the filenames remain the same (the whatever.approved.cs file)?

yes

Changes in the contents of the approved file?

no

Are there any changes in behaviour users will/may see between v3 (the old way) and this? I

Not that i can see

Diff engine does not touch the files. just handles passing them to a diff tool