From e804ba5294fa3013018a843e980bc373f5581bba Mon Sep 17 00:00:00 2001 From: Ryan Willis Date: Fri, 17 Jul 2020 04:03:07 -0700 Subject: [PATCH] vfmt: add support for VDIFF_TOOL, detect more diffing tools (#5857) --- cmd/tools/modules/vhelp/vhelp.v | 2 +- cmd/tools/vfmt.v | 36 +++++++++------ cmd/v/help/fmt.txt | 32 +++++++++---- doc/docs.md | 6 +-- vlib/os/os_windows.c.v | 5 +- vlib/v/util/diff.v | 82 +++++++++++++++++++++++++++++++++ vlib/v/util/errors.v | 62 +++---------------------- vlib/v/util/util.v | 6 +-- 8 files changed, 144 insertions(+), 87 deletions(-) create mode 100644 vlib/v/util/diff.v diff --git a/cmd/tools/modules/vhelp/vhelp.v b/cmd/tools/modules/vhelp/vhelp.v index 9868d0e809..584bb96835 100644 --- a/cmd/tools/modules/vhelp/vhelp.v +++ b/cmd/tools/modules/vhelp/vhelp.v @@ -5,7 +5,7 @@ import os pub fn show_topic(topic string) { vexe := os.real_path(os.getenv('VEXE')) vroot := os.dir(vexe) - target_topic := os.join_path(vroot,'cmd','v','internal','help','${topic}.txt') + target_topic := os.join_path(vroot, 'cmd', 'v', 'help', '${topic}.txt') content := os.read_file(target_topic) or { eprintln('Unknown topic: $topic') exit(1) diff --git a/cmd/tools/vfmt.v b/cmd/tools/vfmt.v index 9b8c3d4e06..d2241b42b6 100644 --- a/cmd/tools/vfmt.v +++ b/cmd/tools/vfmt.v @@ -15,7 +15,7 @@ import vhelp struct FormatOptions { is_l bool - is_c bool // NB: This refers to the '-c' fmt flag, NOT the C backend + is_c bool // NB: This refers to the '-c' fmt flag, NOT the C backend is_w bool is_diff bool is_verbose bool @@ -27,11 +27,18 @@ struct FormatOptions { } const ( - platform_and_file_extensions = [['windows', '_windows.v'], ['linux', '_lin.v', - '_linux.v', '_nix.v'], ['macos', '_mac.v', '_darwin.v'], ['freebsd', '_bsd.v', '_freebsd.v'], - ['netbsd', '_bsd.v', '_netbsd.v'], ['openbsd', '_bsd.v', '_openbsd.v'], ['solaris', '_solaris.v'], - ['haiku', '_haiku.v'], ['qnx', '_qnx.v']] formatted_file_token = '\@\@\@' + 'FORMATTED_FILE: ' + platform_and_file_extensions = [ + ['windows', '_windows.v'], + ['linux', '_lin.v', '_linux.v', '_nix.v'], + ['macos', '_mac.v', '_darwin.v'], + ['freebsd', '_bsd.v', '_freebsd.v'], + ['netbsd', '_bsd.v', '_netbsd.v'], + ['openbsd', '_bsd.v', '_openbsd.v'], + ['solaris', '_solaris.v'], + ['haiku', '_haiku.v'], + ['qnx', '_qnx.v'] + ] ) fn main() { @@ -132,7 +139,7 @@ fn main() { errors++ } if errors > 0 { - eprintln('Encountered a total of: ${errors} errors.') + eprintln('Encountered a total of: $errors errors.') if foptions.is_noerror { exit(0) } @@ -157,9 +164,9 @@ fn (foptions &FormatOptions) format_file(file string) { vfmt_output_path := os.join_path(os.temp_dir(), 'vfmt_' + file_name) os.write_file(vfmt_output_path, formatted_content) if foptions.is_verbose { - eprintln('fmt.fmt worked and ${formatted_content.len} bytes were written to ${vfmt_output_path} .') + eprintln('fmt.fmt worked and $formatted_content.len bytes were written to $vfmt_output_path .') } - eprintln('${formatted_file_token}${vfmt_output_path}') + eprintln('$formatted_file_token$vfmt_output_path') } fn print_compiler_options(compiler_params &pref.Preferences) { @@ -181,15 +188,18 @@ fn (foptions &FormatOptions) post_process_file(file, formatted_file_path string) } if foptions.is_diff { diff_cmd := util.find_working_diff_command() or { - eprintln('No working "diff" CLI command found.') + eprintln(err) return } + if foptions.is_verbose { + eprintln('Using diff command: $diff_cmd') + } println(util.color_compare_files(diff_cmd, file, formatted_file_path)) return } if foptions.is_verify { diff_cmd := util.find_working_diff_command() or { - eprintln('No working "diff" CLI command found.') + eprintln(err) return } x := util.color_compare_files(diff_cmd, file, formatted_file_path) @@ -234,10 +244,10 @@ fn (foptions &FormatOptions) post_process_file(file, formatted_file_path string) print(formatted_fc) } - fn (f FormatOptions) str() string { - return 'FormatOptions{ is_l: $f.is_l' + ' is_w: $f.is_w' + ' is_diff: $f.is_diff' + ' is_verbose: $f.is_verbose' + - ' is_all: $f.is_all' + ' is_worker: $f.is_worker' + ' is_debug: $f.is_debug' + ' }' + return 'FormatOptions{ is_l: $f.is_l, is_w: $f.is_w, is_diff: $f.is_diff, is_verbose: $f.is_verbose,' + + ' is_all: $f.is_all, is_worker: $f.is_worker, is_debug: $f.is_debug, is_noerror: $f.is_noerror,' + + ' is_verify: $f.is_verify" }' } fn file_to_target_os(file string) string { diff --git a/cmd/v/help/fmt.txt b/cmd/v/help/fmt.txt index a07cf4ff08..b9f399db6c 100644 --- a/cmd/v/help/fmt.txt +++ b/cmd/v/help/fmt.txt @@ -1,17 +1,33 @@ Usage: - v [flags] fmt path_to_source.v [path_to_other_source.v] + v fmt [options] path_to_source.v [path_to_other_source.v] Formats the given V source files, then prints their formatted source to stdout. Options: - -c Check if file is already formatted. - If it is not, print filepath, and exit with code 2. - -diff Display only diffs between the formatted source and the original source. - -l List files whose formatting differs from vfmt. - -w Write result to (source) file(s) instead of to stdout. - -debug Print the kinds of encountered AST statements/expressions on stderr. + -c Check if file is already formatted. + If it is not, print filepath, and exit with code 2. + + -diff Display the differences between the formatted source(s) and the original source(s). + This will attempt to find a working `diff` command automatically unless you + specify one with the VDIFF_TOOL environment variable. + + -l List files whose formatting differs from vfmt. + + -w Write result to (source) file(s) instead of to stdout. + + -debug Print the kinds of encountered AST statements/expressions on stderr. + +Environment Variables: + VDIFF_TOOL A command-line tool that will be executed with the original file path + and a temporary formatted file path as arguments. e.g. + `VDIFF_TOOL=opendiff v fmt -diff path/to/file.v` will become: + opendiff path/to/file.v /tmp/v/vfmt_file.v + + VDIFF_OPTIONS A set of command-line options to be sent immediately after the + `diff` command. e.g. + VDIFF_OPTIONS="-W 80 -y" v fmt -diff path/to/file.v /tmp/v/vfmt_file.v NB: vfmt after 2020/04/01 is based on the new AST compiler code, and thus is much faster, and more flexible than before. It may also EAT some of your code for now, so please be careful, and -make frequent BACKUPS. +make frequent BACKUPS. \ No newline at end of file diff --git a/doc/docs.md b/doc/docs.md index 9734d1ad60..f49bba36be 100644 --- a/doc/docs.md +++ b/doc/docs.md @@ -1768,16 +1768,16 @@ To generate documentation use vdoc, for example `v doc net.http`. ## Tools -### vfmt +### v fmt You don't need to worry about formatting your code or setting style guidelines. -`vfmt` takes care of that: +`v fmt` takes care of that: ```v v fmt file.v ``` -It's recommended to set up your editor, so that vfmt runs on every save. +It's recommended to set up your editor, so that `v fmt -w` runs on every save. A vfmt run is usually pretty cheap (takes <30ms). Always run `v fmt -w file.v` before pushing your code. diff --git a/vlib/os/os_windows.c.v b/vlib/os/os_windows.c.v index 60c8fab9ce..b3fc239b37 100644 --- a/vlib/os/os_windows.c.v +++ b/vlib/os/os_windows.c.v @@ -265,8 +265,9 @@ pub fn exec(cmd string) ?Result { C.ExpandEnvironmentStringsW(cmd.to_wide(), voidptr(&command_line), 32768) create_process_ok := C.CreateProcessW(0, command_line, 0, 0, C.TRUE, 0, 0, 0, voidptr(&start_info), voidptr(&proc_info)) if !create_process_ok { - error_msg := get_error_msg(int(C.GetLastError())) - return error('exec failed (CreateProcess): $error_msg cmd: $cmd') + error_num := int(C.GetLastError()) + error_msg := get_error_msg(error_num) + return error('exec failed (CreateProcess) with code $error_num: $error_msg cmd: $cmd') } C.CloseHandle(child_stdin) C.CloseHandle(child_stdout_write) diff --git a/vlib/v/util/diff.v b/vlib/v/util/diff.v new file mode 100644 index 0000000000..a78dea228f --- /dev/null +++ b/vlib/v/util/diff.v @@ -0,0 +1,82 @@ +module util + +import os +import time + +// iterates through a list of known diff cli commands +// and returns it with basic options +pub fn find_working_diff_command() ?string { + env_difftool := os.getenv('VDIFF_TOOL') + env_diffopts := os.getenv('VDIFF_OPTIONS') + mut known_diff_tools := []string{} + if env_difftool.len > 0 { + known_diff_tools << env_difftool + } + known_diff_tools << [ + 'colordiff', 'gdiff', 'diff', 'colordiff.exe', + 'diff.exe', 'opendiff', 'code', 'code.cmd' + ] + // NOTE: code.cmd is the Windows variant of the `code` cli tool + for diffcmd in known_diff_tools { + if diffcmd == 'opendiff' { // opendiff has no `--version` option + if opendiff_exists() { + return diffcmd + } + continue + } + p := os.exec('$diffcmd --version') or { + continue + } + if p.exit_code == 127 && diffcmd == env_difftool { + // user setup is wonky, fix it + return error('could not find specified VDIFF_TOOL $diffcmd') + } + if p.exit_code == 0 { // success + if diffcmd in ['code', 'code.cmd'] { + // there is no guarantee that the env opts exist + // or include `-d`, so (harmlessly) add it + return '$diffcmd $env_diffopts -d' + } + return '$diffcmd $env_diffopts' + } + } + return error('No working "diff" command found') +} + +// determine if the FileMerge opendiff tool is available +fn opendiff_exists() bool { + o := os.exec('opendiff') or { + return false + } + if o.exit_code == 1 { // failed (expected), but found (i.e. not 127) + if o.output.contains('too few arguments') { // got some exptected output + return true + } + } + return false +} + +pub fn color_compare_files(diff_cmd, file1, file2 string) string { + if diff_cmd != '' { + full_cmd := '$diff_cmd --minimal --text --unified=2 ' + + ' --show-function-line="fn " "$file1" "$file2" ' + x := os.exec(full_cmd) or { + return 'comparison command: `$full_cmd` failed' + } + return x.output.trim_right('\r\n') + } + return '' +} + +pub fn color_compare_strings(diff_cmd, expected, found string) string { + cdir := os.cache_dir() + ctime := time.sys_mono_now() + e_file := os.join_path(cdir, '${ctime}.expected.txt') + f_file := os.join_path(cdir, '${ctime}.found.txt') + os.write_file(e_file, expected) + os.write_file(f_file, found) + res := color_compare_files(diff_cmd, e_file, f_file) + os.rm(e_file) + os.rm(f_file) + return res +} diff --git a/vlib/v/util/errors.v b/vlib/v/util/errors.v index 4228aa10ff..3abf82dd3a 100644 --- a/vlib/v/util/errors.v +++ b/vlib/v/util/errors.v @@ -6,7 +6,6 @@ module util import os import term import v.token -import time // The filepath:line:col: format is the default C compiler error output format. // It allows editors and IDE's like emacs to quickly find the errors in the @@ -89,7 +88,7 @@ pub fn formatted_error(kind, omsg, filepath string, pos token.Position) string { } } column := imax(0, pos.pos - p - 1) - position := '${path}:${pos.line_nr+1}:${util.imax(1,column+1)}:' + position := '$path:${pos.line_nr+1}:${imax(1,column+1)}:' scontext := source_context(kind, source, column, pos).join('\n') final_position := bold(position) final_kind := bold(color(kind, kind)) @@ -112,11 +111,8 @@ pub fn source_context(kind, source string, column int, pos token.Position) []str sline := source_lines[iline] start_column := imax(0, imin(column, sline.len)) end_column := imax(0, imin(column + imax(0, pos.len), sline.len)) - cline := if iline == pos.line_nr { - sline[..start_column] + color(kind, sline[start_column..end_column]) + sline[end_column..] - } else { - sline - } + cline := if iline == pos.line_nr { sline[..start_column] + color(kind, sline[start_column..end_column]) + + sline[end_column..] } else { sline } clines << '${iline+1:5d} | ' + cline.replace('\t', tab_spaces) // if iline == pos.line_nr { @@ -126,18 +122,10 @@ pub fn source_context(kind, source string, column int, pos token.Position) []str // use strings.repeat(` `, col) to form it. mut pointerline := '' for bchar in sline[..start_column] { - x := if bchar.is_space() { - bchar - } else { - ` ` - } + x := if bchar.is_space() { bchar } else { ` ` } pointerline += x.str() } - underline := if pos.len > 1 { - '~'.repeat(end_column - start_column) - } else { - '^' - } + underline := if pos.len > 1 { '~'.repeat(end_column - start_column) } else { '^' } pointerline += bold(color(kind, underline)) clines << ' | ' + pointerline.replace('\t', tab_spaces) } @@ -147,44 +135,6 @@ pub fn source_context(kind, source string, column int, pos token.Position) []str pub fn verror(kind, s string) { final_kind := bold(color(kind, kind)) - eprintln('${final_kind}: $s') + eprintln('$final_kind: $s') exit(1) } - -pub fn find_working_diff_command() ?string { - for diffcmd in ['colordiff', 'gdiff', 'diff', 'colordiff.exe', 'diff.exe'] { - p := os.exec('$diffcmd --version') or { - continue - } - if p.exit_code == 0 { - return diffcmd - } - } - return error('no working diff command found') -} - -pub fn color_compare_files(diff_cmd, file1, file2 string) string { - if diff_cmd != '' { - mut other_options := os.getenv('VDIFF_OPTIONS') - full_cmd := '$diff_cmd --minimal --text --unified=2 ' + - ' --show-function-line="fn " $other_options "$file1" "$file2" ' - x := os.exec(full_cmd) or { - return 'comparison command: `${full_cmd}` failed' - } - return x.output.trim_right('\r\n') - } - return '' -} - -pub fn color_compare_strings(diff_cmd string, expected string, found string) string { - cdir := os.cache_dir() - ctime := time.sys_mono_now() - e_file := os.join_path(cdir, '${ctime}.expected.txt') - f_file := os.join_path(cdir, '${ctime}.found.txt') - os.write_file( e_file, expected) - os.write_file( f_file, found) - res := util.color_compare_files(diff_cmd, e_file, f_file) - os.rm( e_file ) - os.rm( f_file ) - return res -} diff --git a/vlib/v/util/util.v b/vlib/v/util/util.v index 11bda1fcaa..0e25c8ffa5 100644 --- a/vlib/v/util/util.v +++ b/vlib/v/util/util.v @@ -12,9 +12,7 @@ pub const ( // math.bits is needed by strconv.ftoa pub const ( - builtin_module_parts = ['math.bits', 'strconv', 'strconv.ftoa', 'hash.wyhash', 'strings', - 'builtin' - ] + builtin_module_parts = ['math.bits', 'strconv', 'strconv.ftoa', 'hash.wyhash', 'strings', 'builtin'] ) pub const ( @@ -167,7 +165,7 @@ pub fn launch_tool(is_verbose bool, tool_name string, args []string) { } if tool_compilation.exit_code != 0 { mut err := 'Permission denied' - if !tool_compilation.output.contains('Permission denied') { + if !tool_compilation.output.contains(err) { err = '\n$tool_compilation.output' } eprintln('cannot compile `$tool_source`: $err')