Improved command line argument parsing (#123)

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).

CHANGELOG.md

@@ -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

src/main.jl

@@ -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
@@ -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
@@ -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)
+            # Remaining arguments must be `-`, files, or directories
-            maybe_expand_directory!(inputfiles, x)
+            first = true
-            for x in argv
+            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
@@ -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")
@@ -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
@@ -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
 
@@ -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)

test/maintests.jl

@@ -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"])
@@ -332,11 +346,14 @@ function maintests(f::R) where {R}
         @test occursin("expected output file argument after `-o`", fd2)
     end
 
-    # runic in.jl -
+    # - only allowed once and only in first position
-    let (rc, fd1, fd2) = runic(["in.jl", "-"])
+    cdtmp() do
-        @test rc == 1
+        for argv in [[".", "-"], ["-", "."], ["in.jl", "-"], ["-", "in.jl"]]
-        @test isempty(fd1)
+            rc, fd1, fd2 = runic(argv)
-        @test occursin("input `-` can not be used with multiple files", fd2)
+            @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?)
@@ -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
@@ -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