Skip to content

Commit

Permalink
Improved command line argument parsing (#123)
Browse files Browse the repository at this point in the history
This patch improves the command line argument parsing to handle more
edge cases related to input files and directories (such as e.g. empty
directories).
  • Loading branch information
fredrikekre authored Dec 13, 2024
1 parent 3de7858 commit 3ace161
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 33 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Runic is now silent by default. Use `--verbose` to enable the verbose file progress
printing from previous releases ([#121]).
### 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]).

## [v1.2.0] - 2024-12-09
### Added
Expand Down
63 changes: 38 additions & 25 deletions src/main.jl
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,6 @@ function print_version()
return
end

function maybe_expand_directory!(outfiles, dir)
if !isdir(dir)
# Assumed to be a file, checked when using it
push!(outfiles, dir)
else
scandir!(outfiles, dir)
end
return
end

# juliac: type-stable output struct (required for juliac but useful in general too)
struct Output{IO}
which::Symbol
Expand Down Expand Up @@ -235,6 +225,8 @@ function main(argv)
check = false
fail_fast = false
line_ranges = typeof(1:2)[]
input_is_stdin = true
multiple_inputs = false

# Parse the arguments
while length(argv) > 0
Expand Down Expand Up @@ -271,19 +263,41 @@ function main(argv)
elseif (m = match(r"^--output=(.+)$", x); m !== nothing)
outputfile = String(m.captures[1]::SubString)
else
# Remaining arguments must be inputfile(s)
maybe_expand_directory!(inputfiles, x)
for x in argv
# Remaining arguments must be `-`, files, or directories
first = true
while true
if x == "-"
return panic("input `-` can not be used with multiple files")
# `-` is only allowed once and only in first position
if length(argv) > 0 || !first
return panic("input `-` can not be combined with other input")
end
push!(inputfiles, x)
input_is_stdin = true
else
input_is_stdin = false
if isdir(x)
scandir!(inputfiles, x)
# Directories are considered to be multiple (potential) inputs even
# if they end up being empty
multiple_inputs = true
else # isfile(x)
push!(inputfiles, x) # Assume it is a file for now
end
end
maybe_expand_directory!(inputfiles, x)
length(argv) == 0 && break
x = popfirst!(argv)
first = false
multiple_inputs = true
end
break
end
end

input_is_stdin = length(inputfiles) == 0 || inputfiles[1] == "-"
# Insert `-` as the input if there were no input files/directories on the command line
if input_is_stdin && length(inputfiles) == 0
@assert !multiple_inputs
push!(inputfiles, "-")
end

# Check the arguments
if inplace && check
Expand All @@ -298,20 +312,16 @@ function main(argv)
if inplace && input_is_stdin
return panic("option `--inplace` can not be used together with stdin input")
end
if outputfile != "" && length(inputfiles) > 1
if outputfile != "" && multiple_inputs
return panic("option `--output` can not be used together with multiple input files")
end
if !isempty(line_ranges) && length(inputfiles) > 1
if !isempty(line_ranges) && multiple_inputs
return panic("option `--lines` can not be used together with multiple input files")
end
if length(inputfiles) > 1 && !(inplace || check)
if multiple_inputs && !(inplace || check)
return panic("option `--inplace` or `--check` required with multiple input files")
end

if length(inputfiles) == 0
push!(inputfiles, "-")
end

if diff
if Sys.which("git") === nothing
return panic("option `--diff` requires `git` to be installed")
Expand All @@ -322,8 +332,10 @@ function main(argv)
nfiles_str = string(length(inputfiles))
for (file_counter, inputfile) in enumerate(inputfiles)
# Read the input
if input_is_stdin
if inputfile == "-"
@assert input_is_stdin
@assert length(inputfiles) == 1
@assert !multiple_inputs
sourcetext = try
read(stdin, String)
catch err
Expand All @@ -338,7 +350,7 @@ function main(argv)
continue
end
else
panic("input file does not exist: `$(inputfile)`")
panic("input path is not a file or directory: `$(inputfile)`")
continue
end

Expand All @@ -353,6 +365,7 @@ function main(argv)
output = Output(:devnull, "", stdout, false, false)
else
@assert length(inputfiles) == 1
@assert !multiple_inputs
if outputfile == "" || outputfile == "-"
output = Output(:stdout, "", stdout, false, false)
elseif isfile(outputfile) && !input_is_stdin && samefile(outputfile, inputfile)
Expand Down
34 changes: 26 additions & 8 deletions test/maintests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,20 @@ function maintests(f::R) where {R}
end
end

# runic emptydir/
cdtmp() do
for argv in [["-i", "."], ["-c", "."], ["-c", "-v", "."], ["-c", "-d", "."]]
rc, fd1, fd2 = runic(["-i", "."])
@test rc == 0
@test isempty(fd1) && isempty(fd2)
end
let (rc, fd1, fd2) = runic(["."])
@test rc == 1
@test isempty(fd1)
@test occursin("option `--inplace` or `--check` required with multiple", fd2)
end
end

# Error paths
# runic -o
let (rc, fd1, fd2) = runic(["-o"])
Expand All @@ -332,11 +346,14 @@ function maintests(f::R) where {R}
@test occursin("expected output file argument after `-o`", fd2)
end

# runic in.jl -
let (rc, fd1, fd2) = runic(["in.jl", "-"])
@test rc == 1
@test isempty(fd1)
@test occursin("input `-` can not be used with multiple files", fd2)
# - only allowed once and only in first position
cdtmp() do
for argv in [[".", "-"], ["-", "."], ["in.jl", "-"], ["-", "in.jl"]]
rc, fd1, fd2 = runic(argv)
@test rc == 1
@test isempty(fd1)
@test occursin("input `-` can not be combined with other input", fd2)
end
end

# runic --inplace --check (TODO: perhaps this should be allowed?)
Expand Down Expand Up @@ -404,10 +421,11 @@ function maintests(f::R) where {R}

# runic doesntexist.jl
cdtmp() do
rc, fd1, fd2 = runic(["doesntexist.jl"])
doesntexist = "doesntexist.jl"
rc, fd1, fd2 = runic([doesntexist])
@test rc == 1
@test isempty(fd1)
@test occursin("input file does not exist", fd2)
@test occursin("input path is not a file or directory: `$doesntexist`", fd2)
end

# runic -o in.jl in.jl
Expand Down Expand Up @@ -481,7 +499,7 @@ function maintests(f::R) where {R}
rc, fd1, fd2 = runic(["--lines=3:4"], src)
@test rc == 1 && isempty(fd1)
@test occursin("`--lines` range out of bounds", fd2)
rc, fd1, fd2 = runic(["--lines=3:4", "foo.jl", "bar.jl"], src)
rc, fd1, fd2 = runic(["--lines=3:4", "."])
@test rc == 1 && isempty(fd1)
@test occursin("option `--lines` can not be used together with multiple input files", fd2)
end
Expand Down

0 comments on commit 3ace161

Please sign in to comment.