feat: Add configuration option for setting number of completion candidates

[majjoha]

After a conversation about adding support for configuring the number of completion candidates in #315, I decided to try implementing it.

It is still work in progress, and I'll update the status once it works, and the tests pass.

[artempyanykh]

Parsing need to be fixed but overall LGTM. Thanks for putting this together 💖

[artempyanykh]

The parsing is a bit wonky. TOML represents numbers as int64 and those are not automatically coercible to int. You can do something like the code below if you want to work with ints.

// TOML parser represents numbers as int64, hence extract as int64 and convert to int
let! complCandidates = getFromTableOpt<int64> table [] [ "completion"; "candidates" ]
let complCandidates = complCandidates |> Option.map int

One potential improvement is to check the the parsed value is >0 which will require extending the LookupError variant to also include a WrongType. The diff might look like this:

git diff
diff --git a/Marksman/Config.fs b/Marksman/Config.fs
index 5e944ef..41ba9d2 100644
--- a/Marksman/Config.fs
+++ b/Marksman/Config.fs
@@ -9,6 +9,7 @@ open Tomlyn.Model
 type LookupError =
     | NotFound of path: list<string>
     | WrongType of path: list<string> * value: obj * expectedType: System.Type
+    | WrongValue of path: list<string> * value: obj * err: string

 type LookupResult<'R> = Result<'R, LookupError>

@@ -215,7 +216,18 @@ let private configOfTable (table: TomlTable) : LookupResult<Config> =
         let complWikiStyle =
             complWikiStyle |> Option.bind ComplWikiStyle.ofStringOpt

-        let! complCandidates = getFromTableOpt<int> table [] [ "completion"; "candidates" ]
+        // TOML parser represents numbers as int64, hence extract as int64 and convert to int
+        let complCandidatesPath = [ "completion"; "candidates" ]
+        let! complCandidates = getFromTableOpt<int64> table [] complCandidatesPath
+
+        let! complCandidates =
+            match complCandidates with
+            | None -> Ok None
+            | Some v ->
+                if v > 0 then
+                    Ok(Some(int v))
+                else
+                    Error(WrongValue(complCandidatesPath, v, "expected a non-negative number"))

         { caTocEnable = caTocEnable
           caCreateMissingFileEnable = caCreateMissingFileEnable
@@ -263,7 +275,13 @@ module Config =

             match configOfTable table with
             | Ok parsed -> Some parsed
-            | _ -> None
+            | err ->
+                logger.error (
+                    Log.setMessage "Failed to parse configuration"
+                    >> Log.addContext "error" err
+                )
+
+                None
         else
             logger.trace (Log.setMessage "Parsing as TOML failed")
             None

[majjoha]

Thank you. This is very, very helpful. :-)

[majjoha]

Thank you for the help, @artempyanykh. I greatly appreciate it!

I've now updated the pull request, so the candidates value is converted into an int and added the check to ensure that it is a positive value. There is also an associated test for this check.

Although, the tests now pass, and everything is getting closer to working, reading the actual value correctly on this line doesn't seem to work. Instead, it defaults to 50 rather than reading the value set in either the user-configured marksman.toml file or in default.marksman.toml. So far, I've tried the following:

// First attempt
let maxCompletions = (State.userConfigOrDefault state).ComplCandidates()

// Second attempt
let maxCompletions =
    match State.tryFindFolderAndDoc docPath state with
    | Some(folder, _) ->
        let config = Folder.configOrDefault folder
        config.ComplCandidates()
    | None ->
        50

// Third attempt
let maxCompletions = tryLoadUserConfig() |> Option.map (fun x -> x.ComplCandidates()) |> Option.defaultValue 50

Neither of them are working, so it seems that I'll have to keep investigating a bit. :-)

[artempyanykh]

I'll have a look soon

[majjoha]

Thank you, and, please, no rush. I really appreciate your help!

[artempyanykh]

Also, please ensure that make fmt && make check finishes ok.

[majjoha]

I've committed your suggestion, and it finally seems to work. I appreciate your patience and help. :-)

[artempyanykh]

Thanks for putting this together!