From 97984adaf3d70da9c5cb1e7f352fb9a289430056 Mon Sep 17 00:00:00 2001 From: Turiiya <34311583+ttytm@users.noreply.github.com> Date: Sun, 5 May 2024 12:12:04 +0200 Subject: [PATCH] v.util: rewrite diff module, deprecate old functions (#21403) --- cmd/tools/vfmt.v | 20 +--- vlib/v/util/diff.v | 6 ++ vlib/v/util/diff/diff.v | 175 +++++++++++++++++++++++++++++++---- vlib/v/util/diff/diff_test.v | 79 ++++++++++++---- 4 files changed, 225 insertions(+), 55 deletions(-) diff --git a/cmd/tools/vfmt.v b/cmd/tools/vfmt.v index de51ad130d..b6869335bc 100644 --- a/cmd/tools/vfmt.v +++ b/cmd/tools/vfmt.v @@ -200,19 +200,6 @@ fn print_compiler_options(compiler_params &pref.Preferences) { eprintln(' is_script: ${compiler_params.is_script} ') } -fn (mut foptions FormatOptions) find_diff_cmd() string { - if foptions.diff_cmd != '' { - return foptions.diff_cmd - } - if foptions.is_verify || foptions.is_diff { - foptions.diff_cmd = diff.find_working_diff_command() or { - eprintln(err) - exit(1) - } - } - return foptions.diff_cmd -} - fn (mut foptions FormatOptions) post_process_file(file string, formatted_file_path string) ! { if formatted_file_path == '' { return @@ -230,12 +217,7 @@ fn (mut foptions FormatOptions) post_process_file(file string, formatted_file_pa if !is_formatted_different { return } - diff_cmd := foptions.find_diff_cmd() - foptions.vlog('Using diff command: ${diff_cmd}') - diff_ := diff.color_compare_files(diff_cmd, file, formatted_file_path) - if diff_.len > 0 { - println(diff_) - } + println(diff.compare_files(file, formatted_file_path)!) return } if foptions.is_verify { diff --git a/vlib/v/util/diff.v b/vlib/v/util/diff.v index ea25b71ecf..95cddca36e 100644 --- a/vlib/v/util/diff.v +++ b/vlib/v/util/diff.v @@ -3,16 +3,22 @@ module util import v.util.diff // find_working_diff_command returns the first available command from a list of known diff cli tools. +@[deprecated_after: '2024-06-30'] +@[deprecated] pub fn find_working_diff_command() !string { return diff.find_working_diff_command() } // color_compare_files returns a colored diff between two files. +@[deprecated: 'use `diff.compare_files` instead'] +@[deprecated_after: '2024-06-30'] pub fn color_compare_files(diff_cmd string, path1 string, path2 string) string { return diff.color_compare_files(diff_cmd, path1, path2) } // color_compare_strings returns a colored diff between two strings. +@[deprecated: 'use `diff.compare_text` instead'] +@[deprecated_after: '2024-06-30'] pub fn color_compare_strings(diff_cmd string, unique_prefix string, expected string, found string) string { return diff.color_compare_strings(diff_cmd, unique_prefix, expected, found) } diff --git a/vlib/v/util/diff/diff.v b/vlib/v/util/diff/diff.v index 890e1f3b21..b4ee48f79b 100644 --- a/vlib/v/util/diff/diff.v +++ b/vlib/v/util/diff/diff.v @@ -3,7 +3,146 @@ module diff import os import time +pub enum DiffTool { + auto + diff // core package on Unix-like systems. + colordiff // `diff` wrapper. + delta // viewer for git and diff output. + // fc // built-in tool on windows. // TODO: enable when its command output can be read. +} + +@[params] +pub struct CompareOptions { +pub: + tool DiffTool + // Custom args used with the diff command. + args string + // Sets the environment variable whose value can overwrite a diff command passed to a compare function. + // It also enables the use of commands that are not in the list of known diff tools. + // Set it to `none` to disable it. + env_overwrite_var ?string = 'VDIFF_CMD' +} + +// Default options for `diff` and `colordiff`. +// Short `diff` args are supported more widely (e.g. on OpenBSD, ref. https://man.openbsd.org/diff.1). +// `-d -a -U 2` ^= `--minimal --text --unified=2` +const default_diff_args = $if openbsd || freebsd { '-d -a -U 2' } $else { '-d -a -U 2 -F "fn "' } +const known_diff_tool_defaults = { + // When searching for an automatically available diff tool, the tools are searched in this order. + DiffTool.delta: '' + .colordiff: default_diff_args + .diff: default_diff_args + // .fc: '/lnt' +} + +// Allows public checking for the available tool and prevents repeated searches +// when using compare functions with automatic diff tool detection. +pub const available_tool = find_working_diff_tool() + +// compare_files returns a string displaying the differences between two files. +pub fn compare_files(path1 string, path2 string, opts CompareOptions) !string { + p1, p2 := os.quoted_path(os.real_path(path1)), os.quoted_path(os.real_path(path2)) + if v := opts.env_overwrite_var { + env_cmd := os.getenv(v) + if env_cmd != '' { + tool, args := env_cmd.split_once(' ') or { env_cmd, opts.args } + os.find_abs_path_of_executable(tool) or { + return error('error: failed to find comparison command `${tool}`') + } + return run_tool('${tool} ${args} ${p1} ${p2}', @LOCATION) + } + } + tool, cmd := opts.find_tool()! + mut args := opts.args + if args == '' { + args = if defaults := diff.known_diff_tool_defaults[tool] { defaults } else { '' } + if opts.tool == .diff { + // Ensure that the diff command supports the color option. + // E.g., some BSD installations or macOS diff (based on FreeBSD diff) + // might not include additional diffutils by default. + res := run_tool('${cmd} ${args} --color=always ${p1} ${p2}', @LOCATION) + if !res.contains('unrecognized option') { + return res + } + } + } + return run_tool('${cmd} ${args} ${p1} ${p2}', @LOCATION) +} + +// compare_text returns a string displaying the differences between two strings. +pub fn compare_text(text1 string, text2 string, opts CompareOptions) !string { + opts.find_tool()! + if text1 == text2 { + return '' + } + ctime := time.sys_mono_now() + tmp_dir := os.join_path_single(os.vtmp_dir(), ctime.str()) + os.mkdir(tmp_dir)! + defer { + os.rmdir_all(tmp_dir) or {} + } + path1 := os.join_path_single(tmp_dir, 'text1.txt') + path2 := os.join_path_single(tmp_dir, 'text2.txt') + // When comparing strings and not files, prevent `\ No newline at end of file` in the output. + if !text1.ends_with('\n') || !text2.ends_with('\n') { + os.write_file(path1, text1 + '\n')! + os.write_file(path2, text2 + '\n')! + } else { + os.write_file(path1, text1)! + os.write_file(path2, text2)! + } + return compare_files(path1, path2, opts)! +} + +fn (opts CompareOptions) find_tool() !(DiffTool, string) { + tool := if opts.tool == .auto { + auto_tool := diff.available_tool or { + return error('error: failed to find comparison command') + } + + auto_tool + } else { + opts.tool + } + cmd := $if windows { '${tool.str()}.exe' } $else { tool.str() } + if opts.tool == .auto { + // At this point it was already ensured that the automatically detected tool is available. + return tool, cmd + } + os.find_abs_path_of_executable(cmd) or { + return error('error: failed to find comparison command `${cmd}`') + } + return tool, cmd +} + +fn find_working_diff_tool() ?DiffTool { + for tool in diff.known_diff_tool_defaults.keys() { + cmd := $if windows { '${tool.str()}.exe' } $else { tool.str() } + os.find_abs_path_of_executable(cmd) or { continue } + if tool == .delta { + // Sanity check that the `delta` executable is actually the diff tool. + res := os.execute_opt('${cmd} --help') or { continue } + help_desc := res.output.trim_space().all_before('\n') + if !help_desc.contains('diff') { + dbg('delta does not appear to be the diff tool `${help_desc}`', @LOCATION) + continue + } + } + return tool + } + return none +} + +fn run_tool(cmd string, dbg_location string) string { + dbg('cmd=`${cmd}`', dbg_location) + res := os.execute(cmd) + dbg('res=`${res}`', dbg_location) + return res.output.trim_right('\r\n') +} + // find_working_diff_command returns the first available command from a list of known diff cli tools. +@[deprecated_after: '2024-06-30'] +@[deprecated] pub fn find_working_diff_command() !string { env_difftool := os.getenv('VDIFF_TOOL') env_diffopts := os.getenv('VDIFF_OPTIONS') @@ -33,28 +172,27 @@ pub fn find_working_diff_command() !string { } // color_compare_files returns a colored diff between two files. +@[deprecated: 'use `compare_files` instead'] +@[deprecated_after: '2024-06-30'] pub fn color_compare_files(diff_cmd string, path1 string, path2 string) string { - cmd := diff_cmd.all_before(' ') - os.find_abs_path_of_executable(cmd) or { return 'comparison command: `${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 "'] - } - if cmd == 'diff' { - color_diff_cmd := '${diff_cmd} --color=always ${flags.join(' ')} ${os.quoted_path(path1)} ${os.quoted_path(path2)}' - color_result := os.execute(color_diff_cmd) - if !color_result.output.starts_with('diff: unrecognized option') { - return color_result.output.trim_right('\r\n') + tool := diff_cmd.all_before(' ') + os.find_abs_path_of_executable(tool) or { return 'comparison command: `${tool}` not found' } + p1, p2 := os.quoted_path(os.real_path(path1)), os.quoted_path(os.real_path(path2)) + if tool == 'diff' { + // Ensure that the diff command supports the color option. + // E.g., some BSD installations do not include `diffutils` as a core package alongside `diff`. + res := os.execute('${diff_cmd} --color=always ${diff.default_diff_args} ${p1} ${p2}') + if !res.output.starts_with('diff: unrecognized option') { + return res.output.trim_right('\r\n') } } - full_cmd := '${diff_cmd} ${flags.join(' ')} ${os.quoted_path(path1)} ${os.quoted_path(path2)}' - return os.execute(full_cmd).output.trim_right('\r\n') + cmd := '${diff_cmd} ${diff.default_diff_args} ${p1} ${p2}' + return os.execute(cmd).output.trim_right('\r\n') } // color_compare_strings returns a colored diff between two strings. +@[deprecated: 'use `compare_text` instead'] +@[deprecated_after: '2024-06-30'] pub fn color_compare_strings(diff_cmd string, unique_prefix string, expected string, found string) string { tmp_dir := os.join_path_single(os.vtmp_dir(), unique_prefix) os.mkdir(tmp_dir) or {} @@ -69,3 +207,8 @@ pub fn color_compare_strings(diff_cmd string, unique_prefix string, expected str res := color_compare_files(diff_cmd, e_file, f_file) return res } + +@[if vdiff_debug ?] +fn dbg(msg string, location string) { + println('[DIFF DEBUG] ${location}: ${msg}') +} diff --git a/vlib/v/util/diff/diff_test.v b/vlib/v/util/diff/diff_test.v index e0a1078150..fe57cfb633 100644 --- a/vlib/v/util/diff/diff_test.v +++ b/vlib/v/util/diff/diff_test.v @@ -1,11 +1,14 @@ import v.util.diff import os +import term const tdir = os.join_path(os.vtmp_dir(), 'diff_test') fn testsuite_begin() { + // Disable environmental overwrites that can result in different compare outputs. + os.setenv('VDIFF_CMD', '', true) os.find_abs_path_of_executable('diff') or { - eprintln('> skipping test, since this test requires `diff` to be installed') + eprintln('> skipping test `${@FILE}`, since this test requires `diff` to be installed') exit(0) } os.mkdir_all(tdir)! @@ -36,39 +39,73 @@ fn test_compare_files() { os.write_file(p1, f1)! os.write_file(p2, f2)! - mut res := diff.color_compare_files('diff', p1, p2) + // Test comparison without specifying a cmd only loosely, since an automatically detected tool + // or can result in a different compare output. + mut res := term.strip_ansi(diff.compare_files(p1, p2)!) + assert res.contains("name: 'Foo'"), res + assert res.contains("name: 'foo'"), res + + // From here on, pass `.diff` via the arg or environment variable to enforce consistent behavior in regular tests. + res = diff.compare_files(p1, p2, tool: .diff)! 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 deprecated + assert res == diff.color_compare_files('diff', p1, p2) // Test again using `find_working_diff_command()`. - os.setenv('VDIFF_TOOL', 'diff', true) - 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 + assert res == diff.color_compare_files(diff.find_working_diff_command()!, p1, p2) - // Test adding a flag via env flag. - os.setenv('VDIFF_OPTIONS', '--ignore-case', true) // ignored, when VDIFF_TOOL is not explicitly set - res = diff.color_compare_files(diff.find_working_diff_command()!, p1, p2) + // Test custom options. + res = diff.compare_files(p1, p2, tool: .diff, args: '-U 2 -i')! 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 deprecated + assert res == term.strip_ansi(diff.color_compare_files('diff --ignore-case', p1, p2)) + + // Test options via env variable. + os.setenv('VDIFF_CMD', 'diff --ignore-case -U 2', true) + defer { + os.setenv('VDIFF_CMD', '', true) + } + res = diff.compare_files(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 deprecated + os.setenv('VDIFF_TOOL', 'diff', true) + os.setenv('VDIFF_OPTIONS', '--ignore-case', true) + assert res == term.strip_ansi(diff.color_compare_files(diff.find_working_diff_command()!, + p1, p2)) + + // Test custom option that interferes with default options. + res = diff.compare_files(p1, p2, tool: .diff, args: '--side-by-side', env_overwrite_var: none)! + assert res.match_glob("*version: '0.0.0'*|*version: '0.1.0'*"), res + + // Test custom diff command. + // Test windows default `fc`. + /* $if windows { // TODO: enable when its `os.execute` output can be read. + res = diff.compare_files(p1, p1, tool: .fc)! + assert res.contains('FC: no differences encountered') + res = diff.compare_files(p1, p2, tool: .fc, args: '/b')! + assert res.contains('FC: ABCD longer than abc') + } */ +} + +fn test_compare_string() { + mut res := diff.compare_text('abc', 'abcd', tool: .diff)! + assert res.contains('-abc'), res + assert res.contains('+abcd'), res + assert !res.contains('No newline at end of file'), res } fn test_coloring() { if os.execute('diff --color=always').output.starts_with('diff: unrecognized option') { - eprintln('> skipping test, since `diff` does not support --color=always') + eprintln('> skipping test `${@FN}`, since `diff` does not support --color=always') return } f1 := 'abc\n' @@ -77,8 +114,10 @@ fn test_coloring() { p2 := os.join_path(tdir, '${@FN}_f2.txt') os.write_file(p1, f1)! os.write_file(p2, f2)! - res := diff.color_compare_files('diff', p1, p2) esc := rune(27) + res := diff.compare_files(p1, p2, tool: .diff)! assert res.contains('${esc}[31m-abc${esc}['), res assert res.contains('${esc}[32m+abcd${esc}['), res + // Test deprecated + assert res == diff.color_compare_files('diff', p1, p2) }