Repository: comonicon/Comonicon.jl (julia)

Issue:
[BUG] Option ordering is messed up because of converting Docstring to a Dictionary
**Describe the bug**
For functions that more than one option, the ordering of the documentation is random. Supposedly, that happens because the data in `CASTED_COMMANDS` is stored as a Dictionary, which does not preserve the doc order.

**Version of the package:**
1.0.0

**To Reproduce**
1. Create a module with the following code:

```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.
"""
@main function main(;x::String, y::String, z::String, g::String)
    print("Hello, world")
end

end
```

2.  Run `import M1; M1.command_main()`

**Expected behavior**
The arguments should follow the order they were defined in.

**Screenshots**
Current results:
![Screenshot from 2022-11-25 22-15-17](https://user-images.githubusercontent.com/6990194/204038107-de2c7d04-986f-4d0f-8f5c-c5d104b49fed.png)

**Potential fix**
If I understand right, the problem is in [`read_options`](https://github.com/comonicon/Comonicon.jl/blob/b8cf0d1d13f127dfa109ead246bc7e8e11b11b8e/src/frontend/markdown.jl#L76). Replacing `Dict` with `OrderedDict` from [OrderedCollections](https://github.com/JuliaCollections/OrderedCollections.jl) should help.

Pull request:
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"])
```

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..d19947b 100644
--- a/src/codegen/julia.jl
+++ b/src/codegen/julia.jl
@@ -533,7 +533,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/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