Skip to content

Added defaultValueMaybe #22

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

Closed
wants to merge 1 commit into from
Closed

Conversation

mrboring
Copy link
Contributor

@mrboring mrboring commented Jul 6, 2025

I typically use both argv and appsettings.json (this provides default values). If the value is not found in argv I then look at appsettings.json and use the default (if it exists). Based on existing code, I've created defaultValueMaybe that will apply a default value if one exists.

Please note that I've removed additional code that is not relevant.

// Proposed new defaultValueMaybe function
module Input =

    /// Sets the default value of an option or argument if it exists.
    let defaultValueMaybe (defaultValue: 'T option) (input: ActionInput<'T>) =
        match defaultValue with
        | Some defaultValue' ->
            input
            |> editOption (fun o -> o.DefaultValueFactory <- (fun _ -> defaultValue'))
            |> editArgument (fun a -> a.DefaultValueFactory <- (fun _ -> defaultValue'))
        | None ->
            input

// Common code
let tryGetDefaultValue (sectionAndName: string) =
    let configNameValue = config[sectionAndName]

    match String.IsNullOrWhiteSpace(configNameValue) with
    | true -> None
    | false -> Some (DirectoryInfo configNameValue)

let description = "Test description"
let aliases = [ "--directory"; "-d" ] 

// Using defaultValueMaybe - I think this is cleaner
option<DirectoryInfo> "directory"
|> Input.description description
|> Input.aliases aliases
|> Input.defaultValueMaybe (tryGetDefaultValue "section1:directory")
|> required
|> validateDirectoryExists

// Using defaultValue
let tempOption =
    option<DirectoryInfo> "directory"
    |> Input.description description
    |> Input.aliases aliases
    |> required
    |> validateDirectoryExists

match tryGetDefaultValue "section1:directory" with
| Some d -> tempOption |> defaultValue d
| None -> tempOption

@JordanMarr
Copy link
Owner

I see your intent, but defaultValueMaybe is a little confusing to me as an API.
It may be better as a util / helper function in your specific project since it makes sense to you for the way you want to use it.

IMO, I think this is easier to understand:

option<DirectoryInfo> "directory"
|> desc description
|> aliases aliases
|> editOption (fun o ->
    tryGetDefaultValue "section1:directory" 
    |> Option.iter (fun dir -> o.DefaultValueFactory <- fun _ -> dir)
)
|> validateDirectoryExists

But if you're doing a lot of those operations, I can see how the helper function would be useful.

Also, I know it's just an example, but one other small note:
I think using required would prevent your default value factory from ever being used as it would return an error if the --directory option was not entered.

@mrboring
Copy link
Contributor Author

mrboring commented Jul 6, 2025

I'll follow your advice and create a library holding extensions to FSharp.SystemCommandLine.

With regard to required, I've had the following results in testing:

  • No entry for --directory in argv, gets value from appsettings.json
  • No entry for --directory in argv or appsettings.json, I get the error: Option 'directory' is required.
  • No entry for --directory in appsettings.json, but argv contains one, gets the value from argv.

So, it looks like required does look at both argv and appsettings.json.

@mrboring mrboring closed this Jul 6, 2025
@JordanMarr
Copy link
Owner

Interesting, and good to know! The S.CL XML docs do not go into that much detail about the behavior. Perhaps I should add that to the required function's XML doc summary.

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

Successfully merging this pull request may close these issues.

2 participants