Skip to content

Commit

Permalink
Improved command line argument parsing
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 committed Dec 12, 2024
1 parent 3de7858 commit 437fec3
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")

Check warning on line 272 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L271-L272

Added lines #L271 - L272 were not covered by tests
end
push!(inputfiles, x)
input_is_stdin = true

Check warning on line 275 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L274-L275

Added lines #L274 - L275 were not covered by tests
else
input_is_stdin = false
if isdir(x)
scandir!(inputfiles, x)

Check warning on line 279 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L279

Added line #L279 was not covered by tests
# Directories are considered to be multiple (potential) inputs even
# if they end up being empty
multiple_inputs = true

Check warning on line 282 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L282

Added line #L282 was not covered by tests
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, "-")

Check warning on line 299 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L298-L299

Added lines #L298 - L299 were not covered by tests
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

Check warning on line 336 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L336

Added line #L336 was not covered by tests
@assert length(inputfiles) == 1
@assert !multiple_inputs

Check warning on line 338 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L338

Added line #L338 was not covered by tests
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)`")

Check warning on line 353 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L353

Added line #L353 was not covered by tests
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

Check warning on line 368 in src/main.jl

View check run for this annotation

Codecov / codecov/patch

src/main.jl#L368

Added line #L368 was not covered by tests
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 437fec3

Please sign in to comment.