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

Add bracket indentation semantics to (* *) #1998

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mbottini
Copy link

@mbottini mbottini commented Apr 1, 2024

Background: ionide/ionide-fsgrammar#209 and ionide/ionide-fsgrammar#210

This comes from a comment regarding a pull request that I made on the ionide-fsgrammar repository, which removes the (* *) bracket pair from the "brackets" field of language-configuration.json. One issue with doing this is that we lose the bracket-like indentation that VSCode provides by default for all bracket pairs. This pull request re-adds the same semantics to the indentation rules.

In other words, when I hit Enter in the following configuration, cursor location represented by the white block:

(*█*)

It should indent the cursor and then put the *) on the next line with the same indentation level as the (* as follows:

(*
    █
*)

When I hit Enter with an unaccompanied (*:

(*█

It should simply indent as follows:

(*
    █

Lastly, an unaccompanied *) should outdent. That is,

    *)█

should become

*)█

Background: ionide/ionide-fsgrammar#209 and ionide/ionide-fsgrammar#210

This comes from a comment regarding a pull request that I made on the
ionide-fsgrammar repository, which removes the `(* *)` bracket pair from
the "brackets" field of language-configuration.json. One issue with
doing this is that we lose the bracket-like indentation that VSCode
provides by default for all bracket pairs. This pull request re-adds the
same semantics to the indentation rules.

In other words, when I hit Enter in the following configuration,
cursor location represented by the white block:

    (*█*)

It should indent the cursor and then put the `*)` on the next line
with the same indentation level as the `(*` as follows:

    (*
        █
    *)

---

When I hit Enter with an unaccompanied `(*`:

    (*█

It should simply indent as follows:

    (*
        █

---

Lastly, an unaccompanied `*)` should outdent. That is,

        *)█

should become

    *)█
@MangelMaxime
Copy link
Contributor

Something to note, is that this feature will only be available if user activate FSharp. smartIndent in their setting.

It is not activated by default because the smart indent feature can feel invasive to people and if I remember correctly we can't support all cases correctly.

@MangelMaxime
Copy link
Contributor

If we merge this PR I suppose we are also going to merge ionide/ionide-fsgrammar#210.

Which means we are going to loose auto-indentation for (* ... *) block (due to the grammar PR).

Perhaps we should always compensate (* ... *) block which would lead to something like that:

Warning

Code below is not tested

let activateSmartIndent = "FSharp.smartIndent" |> Configuration.get false

let indentationRules =
    if activateSmartIndent then
        jsOptions<IndentationRule> (fun o ->
            o.increaseIndentPattern <-
                Regex(
                    """^(\s*(module|type|let|static member|member)\b.*=\s*)$|^(\s*(with get|and set)\b.*=.*)$|^(\s*(if|elif|then|else|static member|member|\(\*)).*$"""
                )

            o.decreaseIndentPattern <- Regex("""^(\s*(else|elif|and|\*\))).*$"""))
    else
        jsOptions<IndentationRule> (fun o ->
            o.increaseIndentPattern <-
                Regex(
                    """^(\s*(\(\*)).*$"""
                )

            o.decreaseIndentPattern <- Regex("""^(\s*(\*\))).*$"""))

let setLanguageConfiguration (triggerNotification: bool) =
    // Config always setted
    let config =
        jsOptions<LanguageConfiguration> (fun o ->
            o.onEnterRules <-
                Some
                <| ResizeArray<OnEnterRule>(
                    [|
                        // Doc single-line comment
                        // Example: ///
                        jsOptions<OnEnterRule> (fun rule ->
                            rule.action <-
                                jsOptions<EnterAction> (fun action ->
                                    action.indentAction <- IndentAction.None
                                    action.appendText <- Some "/// ")

                            rule.beforeText <- Regex("^\s*\/{3}.*$"))

                        |]
                )
            o.indentationRules <- Some indentationRules
            )

Doing do mean that we would fix the highlighting coloring thanks to ionide/ionide-fsgrammar#210 but still have indentation for (* ... *) independently of if the user is using smartIndent making this improvements invisible to him.

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