vfmt: add support for VDIFF_TOOL, detect more diffing tools (#5857)

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)

cmd/tools/vfmt.v

@@ -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' +
+	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_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 {

cmd/v/help/fmt.txt

@@ -1,16 +1,32 @@
 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.
+
+  -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

doc/docs.md

@@ -1777,7 +1777,7 @@ You don't need to worry about formatting your code or setting style guidelines.
 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.

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()))
+		error_num := int(C.GetLastError())
-		return error('exec failed (CreateProcess): $error_msg cmd: $cmd')
+		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)

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
+}

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 {
+		cline := if iline == pos.line_nr { sline[..start_column] + color(kind, sline[start_column..end_column]) +
-			sline[..start_column] + color(kind, sline[start_column..end_column]) + sline[end_column..]
+				sline[end_column..] } else { sline }
-		} 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() {
+				x := if bchar.is_space() { bchar } else { ` ` }
-					bchar
-				} else {
-					` `
-				}
 				pointerline += x.str()
 			}
-			underline := if pos.len > 1 {
+			underline := if pos.len > 1 { '~'.repeat(end_column - start_column) } else { '^' }
-				'~'.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
-}

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_module_parts = ['math.bits', 'strconv', 'strconv.ftoa', 'hash.wyhash', 'strings', 'builtin']
-		'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')