From d33ad622ef97128aaa8e3350683509e4440b4b60 Mon Sep 17 00:00:00 2001
From: Turiiya <34311583+ttytm@users.noreply.github.com>
Date: Thu, 11 Apr 2024 15:34:42 +0200
Subject: [PATCH] v.util: improve color_compare_files 2nd try, add test
 (#21247)

---
 vlib/v/util/diff/diff.v      | 36 ++++++++++-----------
 vlib/v/util/diff/diff_test.v | 62 ++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 18 deletions(-)
 create mode 100644 vlib/v/util/diff/diff_test.v

diff --git a/vlib/v/util/diff/diff.v b/vlib/v/util/diff/diff.v
index da38a51bf9bf01..dbd03be352e34b 100644
--- a/vlib/v/util/diff/diff.v
+++ b/vlib/v/util/diff/diff.v
@@ -38,9 +38,12 @@ pub fn find_working_diff_command() !string {
 			return error('could not find specified VDIFF_TOOL ${diffcmd}')
 		}
 		if p.exit_code == 0 { // success
-			if diffcmd in ['gdiff', 'diff'] {
-				if p.output.contains('GNU diffutils') && env_diffopts == '' {
-					return '${diffcmd} --color=always'
+			// TODO: proper implemenation of --color flag
+			$if !macos {
+				if diffcmd in ['gdiff', 'diff'] {
+					if p.output.contains('GNU diffutils') && env_diffopts == '' {
+						return '${diffcmd} --color=always'
+					}
 				}
 			}
 			if diffcmd in ['code', 'code.cmd'] {
@@ -54,22 +57,19 @@ pub fn find_working_diff_command() !string {
 	return error('No working "diff" command found')
 }
 
-pub fn color_compare_files(diff_cmd string, file1 string, file2 string) string {
-	if diff_cmd != '' {
-		mut full_cmd := '${diff_cmd} --minimal --text --unified=2  --show-function-line="fn " ${os.quoted_path(file1)} ${os.quoted_path(file2)} '
-		$if freebsd {
-			full_cmd = '${diff_cmd} --minimal --text --unified=2  ${os.quoted_path(file1)} ${os.quoted_path(file2)} '
-		}
-		$if openbsd {
-			full_cmd = '${diff_cmd} -d -a -U 2 ${os.quoted_path(file1)} ${os.quoted_path(file2)} '
-		}
-		x := os.execute(full_cmd)
-		if x.exit_code < 0 {
-			return 'comparison command: `${full_cmd}` not found'
-		}
-		return x.output.trim_right('\r\n')
+pub fn color_compare_files(diff_cmd string, path1 string, path2 string) string {
+	os.find_abs_path_of_executable(diff_cmd.all_before(' ')) or {
+		return 'comparison command: `${diff_cmd}` not found'
+	}
+	flags := $if openbsd {
+		['-d', '-a', '-U', '2']
+	} $else $if freebsd {
+		['--minimal', '--text', '--unified=2']
+	} $else {
+		['--minimal', '--text', '--unified=2', '--show-function-line="fn "']
 	}
-	return ''
+	full_cmd := '${diff_cmd} ${flags.join(' ')} ${os.quoted_path(path1)} ${os.quoted_path(path2)}'
+	return os.execute(full_cmd).output.trim_right('\r\n')
 }
 
 pub fn color_compare_strings(diff_cmd string, unique_prefix string, expected string, found string) string {
diff --git a/vlib/v/util/diff/diff_test.v b/vlib/v/util/diff/diff_test.v
new file mode 100644
index 00000000000000..4f70020913b80d
--- /dev/null
+++ b/vlib/v/util/diff/diff_test.v
@@ -0,0 +1,62 @@
+import v.util.diff
+import os
+
+fn test_compare_files() {
+	os.find_abs_path_of_executable('diff') or {
+		eprintln('> skipping test, since this test requires `diff` to be installed.')
+		return
+	}
+
+	f1 := "Module{
+	name: 'Foo'
+	description: 'Awesome V module.'
+	version: '0.0.0'
+	dependencies: []
+}
+"
+	f2 := "Module{
+	name: 'foo'
+	description: 'Awesome V module.'
+	version: '0.1.0'
+	license: 'MIT'
+	dependencies: []
+}
+"
+	tdir := os.join_path(os.vtmp_dir(), 'diff_test')
+	os.mkdir_all(tdir)!
+	defer {
+		os.rmdir_all(tdir) or {}
+	}
+	p1 := os.join_path(tdir, 'f1.txt')
+	p2 := os.join_path(tdir, 'f2.txt')
+	os.write_file(p1, f1)!
+	os.write_file(p2, f2)!
+
+	mut res := diff.color_compare_files('diff', p1, p2)
+	assert res.contains("-\tname: 'Foo'"), res
+	assert res.contains("+\tname: 'foo'"), res
+	assert res.contains("-\tversion: '0.0.0'"), res
+	assert res.contains("+\tversion: '0.1.0'"), res
+	assert res.contains("+\tlicense: 'MIT'"), res
+
+	// Test adding a flag to the command.
+	res = diff.color_compare_files('diff --ignore-case', p1, p2)
+	assert !res.contains("+\tname: 'foo'"), res
+	assert res.contains("-\tversion: '0.0.0'"), res
+	assert res.contains("+\tversion: '0.1.0'"), res
+	assert res.contains("+\tlicense: 'MIT'"), res
+
+	// Test again using `find_working_diff_command()`.
+	res = diff.color_compare_files(diff.find_working_diff_command()!, p1, p2)
+	assert res.contains("-\tversion: '0.0.0'"), res
+	assert res.contains("+\tversion: '0.1.0'"), res
+	assert res.contains("+\tlicense: 'MIT'"), res
+
+	// Test adding a flag via env flag.
+	os.setenv('VDIFF_OPTIONS', '--ignore-case', true)
+	res = diff.color_compare_files(diff.find_working_diff_command()!, p1, p2)
+	assert !res.contains("+\tname: 'foo'"), res
+	assert res.contains("-\tversion: '0.0.0'"), res
+	assert res.contains("+\tversion: '0.1.0'"), res
+	assert res.contains("+\tlicense: 'MIT'"), res
+}