[oBusk]

npm diff --diff-text currently has no effect. The flag sets an option, but opts is not passed into shouldPrintPatch(), so no behaviour is changed.

if (await shouldPrintPatch(filename)) {

Expected behaviour

Diffing a package where foo.png is actually plain text:

$ npm diff --diff-text --diff=pkg@1.0.0 --diff=pkg@1.1.0
diff --git a/foo.png b/foo.png
index v1.0.0..v1.1.0 100644
--- a/foo.png
+++ b/foo.png
@@ -1,1 +1,1 @@
-foo
+bar

Actual behaviour

--diff-text is ignored, so files with binary extensions only ever get the headers:

$ npm diff --diff-text --diff=pkg@1.0.0 --diff=pkg@1.1.0
diff --git a/foo.png b/foo.png
index v1.0.0..v1.1.0 100644
--- a/foo.png
+++ b/foo.png

Why tests didn't catch it

• The libnpmdiff unit test calls shouldPrintPatch directly with { diffText: true }, so it never goes through the call in formatDiff.
• Those same tests used t.resolves(promise, true). I believe the intention was to check that the promise resolved the value true, but looking at resolves(), the second parameter is actually a message. resolves() actually just checks if a promise resolves, not what it resolves.
• The npm diff command test that sets 'diff-text': true uses a fixture with no binary-extension files, so its snapshot is the same either way.

Changes

• format-diff.js: pass opts to shouldPrintPatch. I figured I could have skipped claling shouldPrintPatch completely if the option is set, but I saw that shouldPrintPatch() was designed to accept the options, so assuming that actually passing the options to it is the appropiate fix.
• New format-diff test, binary file using --diff-text option, with a .jpg and diffText: true. It fails without the fix.
• Also fix should-print-patch tests to actually assert the returned value, using equal()

Not in scope

When using --diff-text with actually binary files, they are still decoded as UTF-8, and thus provides a bad diffing, but I assume that is expected since --diff-text is an option to "force" binaries as text.