Repository: comonicon/Comonicon.jl
Pull request #227 (2022-12-01 09:50:40)

Fix ordering of options, flags and subcommands
This PR fixes https://github.com/comonicon/Comonicon.jl/issues/226 . It additionally sorts all subcommands alphabetically. Tests for ordering are also there.

Code to test by hands:
```julia
module M1
using Comonicon

"""
Run cell segmentation

# Options

- `-x`: Name of x column. Overrides the config value.
- `-y`: Name of y column. Overrides the config value.
- `-z`: Name of z column. Overrides the config value.
- `-g`: Name of gene column. Overrides the config value.
"""
@cast function main(;x::String, y::String, z::String, g::String)
    print("Hello, world")
end

@cast function test(;x::String, y::String, z::String, g::String)
    print("Hello, world")
end

@cast function run(;x::String, y::String, z::String, g::String)
    print("Hello, world")
end

@main
end

M1.command_main()
M1.command_main(["main"])
```

codecov[bot]: # [Codecov](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
Base: **83.71**% // Head: **1.03**% // Decreases project coverage by **`-82.67%`** :warning:
> Coverage data is based on head [(`2936333`)](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) compared to base [(`67c7e60`)](https://codecov.io/gh/comonicon/Comonicon.jl/commit/67c7e6068b233ed4f05dfe7089ee776b9a08b5eb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> Patch coverage: 0.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #227       +/-   ##
==========================================
- Coverage   83.71%   1.03%   -82.68%     
==========================================
  Files          21      20        -1     
  Lines        1584    1546       -38     
==========================================
- Hits         1326      16     -1310     
- Misses        258    1530     +1272     
```


| [Impacted Files](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [src/ast/printing.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2FzdC9wcmludGluZy5qbA==) | `0.00% <0.00%> (-87.78%)` | :arrow_down: |
| [src/ast/types.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2FzdC90eXBlcy5qbA==) | `88.88% <ø> (-11.12%)` | :arrow_down: |
| [src/codegen/julia.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvZGVnZW4vanVsaWEuamw=) | `0.00% <0.00%> (-89.14%)` | :arrow_down: |
| [src/frontend/cast.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2Zyb250ZW5kL2Nhc3Quamw=) | `0.00% <0.00%> (-81.63%)` | :arrow_down: |
| [src/frontend/markdown.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2Zyb250ZW5kL21hcmtkb3duLmps) | `0.00% <0.00%> (-94.49%)` | :arrow_down: |
| [src/compat.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBhdC5qbA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [src/exceptions.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2V4Y2VwdGlvbnMuamw=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [src/builder/app.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2J1aWxkZXIvYXBwLmps) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [src/builder/install.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2J1aWxkZXIvaW5zdGFsbC5qbA==) | `0.00% <0.00%> (-93.25%)` | :arrow_down: |
| [src/codegen/zsh.jl](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvZGVnZW4venNoLmps) | `0.00% <0.00%> (-92.60%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/comonicon/Comonicon.jl/pull/227?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).

Roger-luo: I think the main concern of having an extra dependency on OrderedCollections initially was the loading time impact which can be critical for use cases like CLI tools. But this PR seems OK on 1.8 (maybe due to compiler improvements)

```julia
julia> @time using Comonicon #master
  0.085131 seconds (278.01 k allocations: 20.346 MiB)
julia> @time using Comonicon # this PR
  0.084021 seconds (278.05 k allocations: 20.348 MiB)
```


VPetukhov: @Roger-luo , thanks for the comments! I made the changes, and they're ready for a review :)

Patch:
diff --git a/Project.toml b/Project.toml
index f075d0a..2a22cf8 100644
--- a/Project.toml
+++ b/Project.toml
@@ -9,6 +9,7 @@ ExproniconLite = "55351af7-c7e9-48d6-89ff-24e801d99491"
 Libdl = "8f399da3-3557-5675-b5ff-fb832c97cbdb"
 Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
 Markdown = "d6f4376e-aef5-505a-96c1-9c027394607a"
+OrderedCollections = "bac558e1-5e72-5ebc-8fee-abe8a469f55d"
 PackageCompiler = "9b87118b-4619-50d2-8e1e-99f35a4d4d9d"
 Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
 Scratch = "6c6a2e73-6563-6170-7368-637461726353"
diff --git a/src/Comonicon.jl b/src/Comonicon.jl
index be2c261..aed13eb 100644
--- a/src/Comonicon.jl
+++ b/src/Comonicon.jl
@@ -9,6 +9,7 @@ module Comonicon
 using Pkg
 using Markdown
 using ExproniconLite
+using OrderedCollections: OrderedDict
 
 export @cast, @main, cmd_error, cmd_exit, @lazyload
 
diff --git a/src/ast/ast.jl b/src/ast/ast.jl
index 0c3d9a0..adee52f 100644
--- a/src/ast/ast.jl
+++ b/src/ast/ast.jl
@@ -2,6 +2,8 @@ module AST
 
 export ComoniconExpr, Description, LeafCommand, NodeCommand, Entry, Argument, Option, Flag, print_cmd
 
+using OrderedCollections: OrderedDict
+
 include("types.jl")
 include("printing.jl")
 include("utils.jl")
diff --git a/src/ast/types.jl b/src/ast/types.jl
index 1ab0948..defbc36 100644
--- a/src/ast/types.jl
+++ b/src/ast/types.jl
@@ -162,13 +162,13 @@ NodeCommand(;fields_above...)
 """
 Base.@kwdef struct NodeCommand <: ComoniconExpr
     name::String
-    subcmds::Dict{String,Any}
+    subcmds::OrderedDict{String,Any}
     description::Description = Description()
     line::Maybe{LineNumberNode} = nothing
 
     function NodeCommand(name, subcmds, description, line)
         !isempty(subcmds) || error("list of subcommands should not be empty")
-        new(name, subcmds, description, line)
+        new(name, sort!(OrderedDict(subcmds)), description, line)
     end
 end
 
@@ -190,8 +190,8 @@ main-cmd node-cmd leaf-cmd 1 2 3
 - `args::Vector{Argument}`: list of CLI arguments, see [`Argument`](@ref), default is `Argument[]`.
 - `nrequire::Int`: number of required arguments, default is the number of `require==true` arugments in `args`.
 - `vararg::Maybe{Argument}`: variant argument, default is `nothing`.
-- `flags::Dict{String, Flag}`: map between flag name and flag object, see [`Flag`](@ref), default is the empty collection.
-- `options::Dict{String, Option}`: map between option name and option object, see [`Option`](@ref), default is the empty collection.
+- `flags::OrderedDict{String, Flag}`: map between flag name and flag object, see [`Flag`](@ref), default is the empty collection.
+- `options::OrderedDict{String, Option}`: map between option name and option object, see [`Option`](@ref), default is the empty collection.
 - `description::Description`: description of the leaf command, default is `Description()`.
 - `line::Maybe{LineNumberNode}`: line number of the leaf command in original Julia program.
 
@@ -208,8 +208,8 @@ Base.@kwdef struct LeafCommand <: ComoniconExpr
     args::Vector{Argument} = Argument[]
     nrequire::Int = count(x -> x.require, args)
     vararg::Maybe{Argument} = nothing
-    flags::Dict{String,Flag} = Dict{String,Flag}()
-    options::Dict{String,Option} = Dict{String,Option}()
+    flags::OrderedDict{String,Flag} = OrderedDict{String,Flag}()
+    options::OrderedDict{String,Option} = OrderedDict{String,Option}()
     description::Description = Description()
     line::Maybe{LineNumberNode} = nothing
 
diff --git a/src/codegen/julia.jl b/src/codegen/julia.jl
index 758a266..ce0cdc9 100644
--- a/src/codegen/julia.jl
+++ b/src/codegen/julia.jl
@@ -274,8 +274,10 @@ function emit_leaf_call(cmd::LeafCommand, ctx::EmitContext, args::Symbol, kwargs
         for option in values(cmd.options)
             Configurations.is_option(option.type) || continue
             push!(ret.args, quote
-                $kwargs[$(QuoteNode(option.sym))] =
-                    $Configurations.from_dict($(option.type), $kwargs[$(QuoteNode(option.sym))])
+                if $(QuoteNode(option.sym)) in Base.keys($kwargs)
+                    $kwargs[$(QuoteNode(option.sym))] =
+                        $Configurations.from_dict($(option.type), $kwargs[$(QuoteNode(option.sym))])
+                end
             end)
         end
     end
@@ -533,7 +535,7 @@ function emit_option_with_option_type(
     assign_field_to_dict = foreach_leaf_field(option.type, value, fields) do dict_ex, type
         quote
             $field_value = $(emit_parse_value(option, ctx, type, field_value))
-            $dict_ex = $field_value 
+            $dict_ex = $field_value
         end
     end
     assign_field_to_dict.otherwise = emit_error(option, ctx, :("cannot find field $($fields)"))
diff --git a/src/frontend/cast.jl b/src/frontend/cast.jl
index 5aad66a..3330290 100644
--- a/src/frontend/cast.jl
+++ b/src/frontend/cast.jl
@@ -612,7 +612,7 @@ function cast_args(doc::JLMD, args::Vector{JLArgument}, line)
 end
 
 function cast_flags(doc::JLMD, flags::Vector{JLFlag}, line)
-    cmd_flags = Dict{String,Flag}()
+    cmd_flags = OrderedDict{String,Flag}()
     for each in flags
         name = replace(string(each.name), '_' => '-')
         if haskey(doc.flags, name)
@@ -637,7 +637,7 @@ function cast_flags(doc::JLMD, flags::Vector{JLFlag}, line)
 end
 
 function cast_options(doc::JLMD, options::Vector{JLOption}, line)
-    cmd_options = Dict{String,Option}()
+    cmd_options = OrderedDict{String,Option}()
     for each in options
         name = replace(string(each.name), '_' => '-')
         if haskey(doc.options, name)
diff --git a/src/frontend/markdown.jl b/src/frontend/markdown.jl
index 8ad317a..00ea4d6 100644
--- a/src/frontend/markdown.jl
+++ b/src/frontend/markdown.jl
@@ -44,7 +44,7 @@ function read_intro(md::Markdown.MD)
 end
 
 function read_arguments(md::Markdown.MD)
-    args = Dict{String,String}()
+    args = OrderedDict{String,String}()
     sec = read_section(md, ["Arguments", "Args"])
     isempty(sec) && return args
     length(sec) == 1 || error("section Arguments/Args can only have one paragraph")
@@ -58,7 +58,7 @@ function read_arguments(md::Markdown.MD)
 end
 
 function read_flags(md::Markdown.MD)
-    flags = Dict{String,JLMDFlag}()
+    flags = OrderedDict{String,JLMDFlag}()
     sec = read_section(md, "Flags")
     isempty(sec) && return flags
     length(sec) == 1 || error("section Flags can only have one paragraph")
@@ -74,7 +74,7 @@ function read_flags(md::Markdown.MD)
 end
 
 function read_options(md::Markdown.MD)
-    options = Dict{String,JLMDOption}()
+    options = OrderedDict{String,JLMDOption}()
     sec = read_section(md, "Options")
     isempty(sec) && return options
     length(sec) == 1 || error("section Options can only have one paragraph")
diff --git a/src/frontend/types.jl b/src/frontend/types.jl
index 8e55ec0..9a12cb8 100644
--- a/src/frontend/types.jl
+++ b/src/frontend/types.jl
@@ -31,7 +31,7 @@ end
 Base.@kwdef struct JLMD
     desc::String = ""
     intro::String = ""
-    arguments::Dict{String,String} = Dict{String,String}()
-    options::Dict{String,JLMDOption} = Dict{String,JLMDOption}()
-    flags::Dict{String,JLMDFlag} = Dict{String,JLMDFlag}()
+    arguments::OrderedDict{String,String} = OrderedDict{String,String}()
+    options::OrderedDict{String,JLMDOption} = OrderedDict{String,JLMDOption}()
+    flags::OrderedDict{String,JLMDFlag} = OrderedDict{String,JLMDFlag}()
 end


Test patch:
diff --git a/test/codegen/julia/config.jl b/test/codegen/julia/config.jl
index 2421413..bd4161f 100644
--- a/test/codegen/julia/config.jl
+++ b/test/codegen/julia/config.jl
@@ -5,8 +5,8 @@ using Comonicon
 using Configurations
 
 @option struct OptionA
-    a::Int
-    b::Int
+    a::Int = 2
+    b::Int = 2
 end
 
 @option struct OptionB
@@ -19,21 +19,31 @@ end
 
 - `-c, --config <path/to/option/or/specific field>`: config.
 """
-@main function run(;config::OptionB)
+@cast function run(;config::OptionB)
     @test config == OptionB(OptionA(1, 1), 1)
 end
 
+@cast function rundef(;config::OptionA=OptionA())
+    @test config == OptionA(2, 2)
+end
+
+@main
+
 @testset "config options" begin
-    TestConfigOption.command_main(["--config.c=1", "--config.option.a=1", "--config.option.b=1"])
+    TestConfigOption.command_main(["run", "--config.c=1", "--config.option.a=1", "--config.option.b=1"])
 
     opt = TestConfigOption.OptionB(TestConfigOption.OptionA(1, 1), 1)
     to_toml("config.toml", opt)
-    TestConfigOption.command_main(["--config", "config.toml"])
-    TestConfigOption.command_main(["-c", "config.toml"])
+    TestConfigOption.command_main(["run", "--config", "config.toml"])
+    TestConfigOption.command_main(["run", "-c", "config.toml"])
 
     opt = TestConfigOption.OptionB(TestConfigOption.OptionA(1, 1), 2)
     to_toml("config.toml", opt)
-    TestConfigOption.command_main(["--config", "config.toml", "--config.c=1"])
+    TestConfigOption.command_main(["run", "--config", "config.toml", "--config.c=1"])
+
+    TestConfigOption.command_main(["rundef"])
+    TestConfigOption.command_main(["rundef", "--config.a=2"])
+    TestConfigOption.command_main(["rundef", "--config.a=2", "--config.b=2"])
 end
 
 end
diff --git a/test/frontend/cast.jl b/test/frontend/cast.jl
index c5a0f80..77cc5eb 100644
--- a/test/frontend/cast.jl
+++ b/test/frontend/cast.jl
@@ -96,6 +96,12 @@ function foo(
     @test cmd.options["option2"].type === Int
     @test cmd.description == Description("a test function.")
     @test cmd.line == LineNumberNode(0)
+
+    @show cmd.options
+    @test all(keys(cmd.options) .== ["option1", "o", "option2", "option3"])
+
+    @show cmd.flags
+    @test all(keys(cmd.flags) .== ["flag1", "f", "flag2"])
 end
 
 @testset "split_leaf_command" begin
diff --git a/test/frontend/markdown.jl b/test/frontend/markdown.jl
index 6990616..057737a 100644
--- a/test/frontend/markdown.jl
+++ b/test/frontend/markdown.jl
@@ -181,6 +181,15 @@ end
     @test doc.arguments["arg1"] == "argument 1."
     @test doc.flags["long"] == JLMDFlag("long flag.", false)
     @test doc.options["long"] == JLMDOption(nothing, "long option using default hint.", false)
+
+    args_sorted = ["arg1", "arg2", "arg3", "arg4"]
+    @test all(keys(doc.arguments) .== args_sorted)
+
+    opts_sorted = ["short", "short-space", "short-assign", "long", "long-space", "long-assign", "short-underscore"]
+    @test all(keys(doc.options) .== opts_sorted)
+
+    flags_sorted = ["short", "short-space", "long", "long-space"]
+    @test all(keys(doc.flags) .== flags_sorted)
 end
 
 @testset "reverse order" begin