Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved command line argument parsing #123

Merged
merged 1 commit into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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
Loading