From 24c21b722d481176ae841b2cfc269b0e1a4376bf Mon Sep 17 00:00:00 2001 From: Fredrik Ekre Date: Fri, 13 Dec 2024 02:33:35 +0100 Subject: [PATCH] Simplify --verbose output This patch simplifies the verbose output a bit and fixes various cases where an error message would be printed before the file info line instead of after. --- CHANGELOG.md | 2 ++ src/main.jl | 88 +++++++++++++++++++++-------------------------- test/maintests.jl | 61 +++++++++++++++++++++++--------- 3 files changed, 86 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afeb9ff..db3b098 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Improved the command line argument parsing to handle more edge cases related to input files and directories such as e.g. empty directories ([#123]). + - Fix some error paths where the error would be printed before the file info + output in `--verbose` mode ([#124]). ## [v1.2.0] - 2024-12-09 ### Added diff --git a/src/main.jl b/src/main.jl index b16ca78..634cd56 100644 --- a/src/main.jl +++ b/src/main.jl @@ -85,15 +85,12 @@ function panic( return errno end -blue(str) = printstyled(stderr, str; color = :blue) -function okln(str) - blue(str) +function okln() printstyled(stderr, "✔"; color = :green, bold = true) println(stderr) return end -function errln(str) - blue(str) +function errln() printstyled(stderr, "✖"; color = :red, bold = true) println(stderr) return @@ -313,6 +310,7 @@ function main(argv) return panic("option `--inplace` can not be used together with stdin input") end if outputfile != "" && multiple_inputs + # TODO: Why not? return panic("option `--output` can not be used together with multiple input files") end if !isempty(line_ranges) && multiple_inputs @@ -328,9 +326,31 @@ function main(argv) end end + # Disable verbose if piping from/to stdin/stdout + output_is_stdout = !inplace && !check && (outputfile == "" || outputfile == "-") + print_progress = verbose && !(input_is_stdin || output_is_stdout) + # Loop over the input files nfiles_str = string(length(inputfiles)) for (file_counter, inputfile) in enumerate(inputfiles) + + if print_progress + @assert inputfile != "-" + input_pretty = relpath(inputfile) + if Sys.iswindows() + input_pretty = replace(input_pretty, "\\" => "/") + end + prefix = string( + "[", lpad(string(file_counter), textwidth(nfiles_str), " "), "/", + nfiles_str, "] " + ) + verb = check ? "Checking" : "Formatting" + str = string(prefix, verb, " `", input_pretty, "` ") + ndots = 80 - textwidth(str) - 1 - 1 + dots = ndots > 0 ? "."^ndots : "" + printstyled(stderr, string(str, dots, " "); color = :blue) + end + # Read the input if inputfile == "-" @assert input_is_stdin @@ -339,17 +359,21 @@ function main(argv) sourcetext = try read(stdin, String) catch err - return panic("could not read input from stdin: ", err) + print_progress && errln() + panic("could not read input from stdin: ", err) + continue end elseif isfile(inputfile) @assert !input_is_stdin sourcetext = try read(inputfile, String) catch err + print_progress && errln() panic("could not read input from file `$(inputfile)`: ", err) continue end else + print_progress && errln() panic("input path is not a file or directory: `$(inputfile)`") continue end @@ -369,55 +393,21 @@ function main(argv) if outputfile == "" || outputfile == "-" output = Output(:stdout, "", stdout, false, false) elseif isfile(outputfile) && !input_is_stdin && samefile(outputfile, inputfile) - return panic("can not use same file for input and output, use `-i` to modify a file in place") + print_progress && errln() + panic("can not use same file for input and output, use `-i` to modify a file in place") + continue else output = Output(:file, outputfile, stdout, true, false) end end - # Print file info if `verbose` unless piping from/to stdin/stdout - print_progress = verbose && !(input_is_stdin || !(output.output_is_file || check)) - - # Print file info unless quiet and unless input/output is stdin/stdout - if print_progress - @assert inputfile != "-" - input_pretty = relpath(inputfile) - prefix = string( - "[", lpad(string(file_counter), textwidth(nfiles_str), " "), "/", - nfiles_str, "] " - ) - if Sys.iswindows() - input_pretty = replace(input_pretty, "\\" => "/") - end - if check - str = string(prefix, "Checking `", input_pretty, "` ") - ndots = 80 - textwidth(str) - 1 - 1 - dots = ndots > 0 ? "."^ndots : "" - str = string(str, dots, " ") - else - if output.output_is_samefile - output_pretty = " " - else - output_pretty = relpath(output.file) - if Sys.iswindows() - output_pretty = replace(output_pretty, "\\" => "/") - end - output_pretty = " -> `$(output_pretty)` " - end - str = string(prefix, "Formatting `", input_pretty, "`", output_pretty) - ndots = 80 - textwidth(str) - 1 - 1 - dots = ndots > 0 ? "."^ndots : "" - str = string(str, dots, " ") - end - end - # Call the library to format the text ctx = try ctx′ = Context(sourcetext; quiet, verbose, debug, diff, check, line_ranges) format_tree!(ctx′) ctx′ catch err - print_progress && errln(str) + print_progress && errln() if err isa JuliaSyntax.ParseError panic("failed to parse input: ", err) continue @@ -445,23 +435,23 @@ function main(argv) changed = !nodes_equal(ctx.fmt_tree, ctx.src_tree) if check if changed - print_progress && errln(str) + print_progress && errln() global errno = 1 else - print_progress && okln(str) + print_progress && okln() end elseif changed || !inplace @assert output.which !== :devnull try writeo(output, seekstart(ctx.fmt_io)) catch err - print_progress && errln(str) + print_progress && errln() panic("could not write to output file `$(output.file)`: ", err) continue end - print_progress && okln(str) + print_progress && okln() else - print_progress && okln(str) + print_progress && okln() end if changed && diff mktempdir() do dir diff --git a/test/maintests.jl b/test/maintests.jl index adbc9fb..369cded 100644 --- a/test/maintests.jl +++ b/test/maintests.jl @@ -104,7 +104,7 @@ function maintests(f::R) where {R} rc, fd1, fd2 = runic(argv) @test rc == 0 @test isempty(fd1) - @test occursin("[1/1] Formatting `in.jl` -> `out.jl` ...", fd2) + @test occursin("[1/1] Formatting `in.jl` ...", fd2) @test occursin("✔", fd2) @test !occursin("✖", fd2) @test read(f_out, String) == good @@ -411,31 +411,60 @@ function maintests(f::R) where {R} write(f_in, bad) omode = filemode(f_in) chmod(f_in, omode & (typemax(omode) ⊻ 0o444)) - rc, fd1, fd2 = runic([f_in]) + let (rc, fd1, fd2) = runic([f_in]) + @test rc == 1 + @test isempty(fd1) + @test !occursin("Formatting", fd2) + @test occursin("could not read input from file", fd2) + @test occursin("SystemError: opening file", fd2) + end + let (rc, fd1, fd2) = runic(["-v", "-i", f_in]) + @test rc == 1 + @test isempty(fd1) + @test occursin("[1/1] Formatting `in.jl` ...", fd2) + @test occursin("could not read input from file", fd2) + @test occursin("SystemError: opening file", fd2) + end chmod(f_in, omode) - @test rc == 1 - @test isempty(fd1) - @test occursin("could not read input from file", fd2) - @test occursin("SystemError: opening file", fd2) end - # runic doesntexist.jl + # runic doesntexist.jl doesntexist/ cdtmp() do - doesntexist = "doesntexist.jl" - rc, fd1, fd2 = runic([doesntexist]) - @test rc == 1 - @test isempty(fd1) - @test occursin("input path is not a file or directory: `$doesntexist`", fd2) + nofile = "doesntexist.jl" + nodir = "doesntexist.jl" + let (rc, fd1, fd2) = runic(["-c", nofile, nodir]) + @test rc == 1 + @test isempty(fd1) + @test !occursin("Formatting", fd2) + @test occursin("input path is not a file or directory: `$nofile`", fd2) + @test occursin("input path is not a file or directory: `$nodir`", fd2) + end + let (rc, fd1, fd2) = runic(["-v", "-c", nofile, nodir]) + @test rc == 1 + @test isempty(fd1) + @test occursin("[1/2] Checking `$nofile` ...", fd2) + @test occursin("input path is not a file or directory: `$nofile`", fd2) + @test occursin("[2/2] Checking `$nodir` ...", fd2) + @test occursin("input path is not a file or directory: `$nodir`", fd2) + end end # runic -o in.jl in.jl cdtmp() do f_in = "in.jl" write(f_in, bad) - rc, fd1, fd2 = runic(["-o", f_in, f_in]) - @test rc == 1 - @test isempty(fd1) - @test occursin("can not use same file for input and output", fd2) + let (rc, fd1, fd2) = runic(["-o", f_in, f_in]) + @test rc == 1 + @test isempty(fd1) + @test !occursin("Formatting", fd2) + @test occursin("can not use same file for input and output", fd2) + end + let (rc, fd1, fd2) = runic(["-o", f_in, "-v", f_in]) + @test rc == 1 + @test isempty(fd1) + @test occursin("[1/1] Formatting `in.jl` ...", fd2) + @test occursin("can not use same file for input and output", fd2) + end end # runic --check unparseable.jl