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

Bug with the dig function #7

Open
mac10688 opened this issue Oct 13, 2020 · 2 comments
Open

Bug with the dig function #7

mac10688 opened this issue Oct 13, 2020 · 2 comments

Comments

@mac10688
Copy link

  let rec private dig argv (cmd: Command<int>) =
    let config = (cmd.config CommandInfo.empty)
    match argv with
      | [] -> (cmd, [])
      | args ->
        match config.options |> List.choose (fun o -> o.isMatch args)
                             |> List.tryHead with
          | Some rest -> dig rest cmd
          | None ->
            match config.subcommands |> List.tryFind (fun sc -> sc.Summary().name = List.head args) with
              | Some sc -> dig (List.tail args) sc
              | None -> (cmd, args)

I think

        match config.options |> List.choose (fun o -> o.isMatch args)
                             |> List.tryHead with
          | Some rest -> dig rest cmd

will be the cause of infinite recursion.

Really I am trying to write a unit test for the generator function, which calls the dig method and I'm trying to rationalize what it's supposed to do. I don't understand how isMatch is supposed to work. It sounds like something that would return a boolean but instead it returns an option of list of strings?

Here is my first pass at seeing how the generator function works and it hit infinite recursion.

[<Test>]
let generator () =

    let commandSummary  = {
        name = "Name"
        displayName = Some "DisplayName"
        description = "Test description"
        paramNames = None
        help = None
        genSuggestions = fun _ -> []
    }

    let commandOption1 = {
        names = ["commandOptionName1"]
        description = "description"
        isFlag = false
        paramNames = [["paramNames1"]]
        isMatch = fun _ -> Some ["Matches"]
        genSuggestions = fun _ -> []
    }

    let commandOption2 = {
        names = ["commandOptionName2"]
        description = "description"
        isFlag = false
        paramNames = [["paramNames2"]]
        isMatch = fun _ -> Some ["Matches"]
        genSuggestions = fun _ -> []
    }

    let command1 = {
        config = fun _ -> {
            summary = {
                name = "SubName1"
                displayName = Some "SubDisplayName1"
                description = "Sub Test description 1"
                paramNames = Some ["SubTest1"; "SubParams1"]
                help = None
                genSuggestions = fun _ -> []
            }
            options = []
            subcommands = []
        }
        func = fun args -> (0, args)
    }

    let command2 = {
        config = fun _ -> {
            summary = {
                name = "SubName2"
                displayName = Some "SubDisplayName2"
                description = "Sub Test description 2"
                paramNames = Some ["SubTest2"; "SubParams2"]
                help = None
                genSuggestions = fun _ -> []
            }
            options = []
            subcommands = []
        }
        func = fun args -> (0, args)
    }

    let commandInfo = {
        summary = commandSummary
        options = [commandOption1; commandOption2]
        subcommands = [command1; command2]
    }

    let cmd = {
        config = fun _ -> commandInfo
        func = fun args -> (5, args)
    }

    let x = Generators.Help.generate [""] cmd 
    x |> should equal [""]
@cannorin
Copy link
Owner

Nice catch.

isMatch (I don't remember why I used such a bad name!) is supposed to take a list of arguments and return Some restOfTheArguments if the option matches the input (with the restOfTheArguments containing the arguments the option does not use), or None otherwise. The basic idea here is that the list of arguments should reduce its length when an option matches.

I assumed it is always implemented here: https://github.com/cannorin/FSharp.CommandLine/blob/master/src/FSharp.CommandLine/options.fs#L43-L52

And since ICommandOption><_>.Config always use the above implementation: https://github.com/cannorin/FSharp.CommandLine/blob/master/src/FSharp.CommandLine/options.fs#L57-L61, there should be no chance that a hand-written isMatch would be used.

Here, the part isMatch = fun _ -> Some ["Matches"] is causing the infinite loop, since the argument list would always be ["Matches"].

Do you feel this library should be heavily refactored? I'm actually thinking of it.

@mac10688
Copy link
Author

I would like to fully understand the code base and what it's capable of before I judge. That's why I started writing these unit tests, because reading through it I couldn't understand anything and as you mentioned elsewhere it could use a lot more documentation.

It may take me a few months to finish my unit tests to create my PR. After I finish my unit tests, I'll take the opportunity to judge the framework and think of suggestions.

Overall though, I think you have a good vision for what needs to happen to make it better and I'm hoping I can help anyway I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants