Repository: fredrikekre/Runic.jl (julia)

Issue:
Alignment
Currently Runic destroys alignment because it is very stringent about spacing, for example
```julia
a  = 1
ab = 2
```
will be replaced with
```julia
a = 1
ab = 2
```
Doing this is of course a valid option (and what e.g. gofmt does).

The current rule is *a single space around operators*, but I wonder if relaxing it to *at least a single space around operators* would satisfy the "human need" to prettify things with alignment would be a good option. I think this would work in all situations where you typically align things.

Pull request:
This patch implements `# runic: on` and `# runic: off` toggle comments that can be included in the source to toggle formatting on/off.

The two comments i) must be placed on their own lines, ii) must be on the same level in the expression tree, and iii) must come in pairs. An exception to condition iii) is made for top level toggle comments so that formatting for a whole file can be disabled by a `# runic: off` comment at the top without having to add one also at the end of the file.

For compatibility with JuliaFormatter, `#! format: (on|off)` is also
supported but it is not possible to pair e.g. a `# runic: off` comment
with a `#! format: on` comment.

Closes #12, closes #41.

Patch:
diff --git a/README.md b/README.md
index 7875e06..77bd29e 100644
--- a/README.md
+++ b/README.md
@@ -233,13 +233,14 @@ exec 1>&2
 # Run Runic on added and modified files
 git diff-index -z --name-only --diff-filter=AM master | \
     grep -z '\.jl$' | \
-    xargs -0 --no-run-if-empty -p julia --project=@runic -m Runic --check --diff
+    xargs -0 --no-run-if-empty julia --project=@runic -m Runic --check --diff
 ```
 
 ## Formatting specification
 
 This is a list of things that Runic currently is doing:
 
+ - [Toggle formatting](#toggle-formatting)
  - [Line width limit](#line-width-limit)
  - [Indentation](#indentation)
  - [Spaces around operators, assignment, etc](#spaces-around-operators-assignment-etc)
@@ -253,6 +254,41 @@ This is a list of things that Runic currently is doing:
  - [Braces around right hand side of `where`](#braces-around-right-hand-side-of-where)
  - [Whitespace miscellaneous](#whitespace-miscellaneous)
 
+### Toggle formatting
+
+It is possible to toggle formatting around expressions where you want to disable Runic's
+formatting. This can be useful in cases where manual formatting increase the readability of
+the code. For example, manually aligned array literals may look worse when formatted by
+Runic.
+
+The source comments `# runic: off` and `# runic: on` will toggle the formatting off and on,
+respectively. The comments must be on their own line, they must be on the same level in the
+syntax tree, and they must come in pairs.
+
+> [!NOTE]
+> For compatibility with [JuliaFormatter](https://github.com/domluna/JuliaFormatter.jl) the
+> comments `#! format: off` and `#! format: on` are also recognized by Runic.
+
+For example, the following code will toggle off the formatting for the array literal `A`:
+
+```julia
+function foo()
+    a = rand(2)
+    # runic: off
+    A = [
+        -1.00   1.41
+         3.14  -4.05
+    ]
+    # runic: on
+    return A * a
+end
+```
+
+An exception to the pairing rule is made at top level where a `# runic: off` comment will
+disable formatting for the remainder of the file. This is so that a full file can be
+excluded from formatting without having to add a `# runic: on` comment at the end of the
+file.
+
 ### Line width limit
 
 No. Use your <kbd>Enter</kbd> key or refactor your code.
diff --git a/src/Runic.jl b/src/Runic.jl
index d3fe2a5..8b78d48 100644
--- a/src/Runic.jl
+++ b/src/Runic.jl
@@ -130,6 +130,7 @@ mutable struct Context
     # Global state
     indent_level::Int # track (hard) indentation level
     call_depth::Int # track call-depth level for debug printing
+    format_on::Bool
     # Current state
     # node::Union{Node, Nothing}
     prev_sibling::Union{Node, Nothing}
@@ -165,9 +166,11 @@ function Context(
     call_depth = 0
     prev_sibling = next_sibling = nothing
     lineage_kinds = JuliaSyntax.Kind[]
+    format_on = true
     return Context(
         src_str, src_tree, src_io, fmt_io, fmt_tree, quiet, verbose, assert, debug, check,
-        diff, filemode, call_depth, indent_level, prev_sibling, next_sibling, lineage_kinds,
+        diff, filemode, indent_level, call_depth, format_on, prev_sibling, next_sibling,
+        lineage_kinds,
     )
 end
 
@@ -198,6 +201,96 @@ function replace_bytes!(ctx::Context, bytes::Union{String, AbstractVector{UInt8}
     return replace_bytes!(ctx.fmt_io, bytes, Int(sz))
 end
 
+# Validate the toggle comments
+function validate_toggle(ctx, kids, i)
+    toplevel = length(ctx.lineage_kinds) == 1 && ctx.lineage_kinds[1] === K"toplevel"
+    valid = true
+    prev = get(kids, i - 1, nothing)
+    if prev === nothing
+        valid &= toplevel && i == 1
+    else
+        valid &= kind(prev) === K"NewlineWs" || (toplevel && i == 1 && kind(prev) === K"Whitespace")
+    end
+    next = get(kids, i + 1, nothing)
+    if next === nothing
+        valid &= toplevel && i == lastindex(kids)
+    else
+        valid &= kind(next) === K"NewlineWs"
+    end
+    return valid
+end
+
+function check_format_toggle(ctx::Context, node::Node, kid::Node, i::Int)::Union{Int, Nothing}
+    @assert ctx.format_on
+    @assert !is_leaf(node)
+    kids = verified_kids(node)
+    @assert kid === kids[i]
+    # Check if the kid is a comment
+    kind(kid) === K"Comment" || return nothing
+    # Check the comment content
+    reg = r"^#(!)? (runic|format): (on|off)$"
+    str = String(read_bytes(ctx, kid))
+    offmatch = match(reg, str)
+    offmatch === nothing && return nothing
+    toggle = offmatch.captures[3]::AbstractString
+    if toggle == "on"
+        @debug "Ignoring `$(offmatch.match)` toggle since formatting is already on."
+        return nothing
+    end
+    if !validate_toggle(ctx, kids, i)
+        @debug "Ignoring `$(offmatch.match)` toggle since it is not on a separate line."
+        return nothing
+    end
+    # Find a matching closing toggle
+    pos = position(ctx.fmt_io)
+    accept_node!(ctx, kid)
+    for j in (i + 1):length(kids)
+        lkid = kids[j]
+        if kind(lkid) !== K"Comment"
+            accept_node!(ctx, lkid)
+            continue
+        end
+        str = String(read_bytes(ctx, lkid))
+        onmatch = match(reg, str)
+        if onmatch === nothing
+            accept_node!(ctx, lkid)
+            continue
+        end
+        # Check that the comments match in style
+        if offmatch.captures[1] != onmatch.captures[1] ||
+                offmatch.captures[2] != onmatch.captures[2]
+            @debug "Ignoring `$(onmatch.match)` toggle since it doesn't match the " *
+                "style of the `$(offmatch.match)` toggle."
+            accept_node!(ctx, lkid)
+            continue
+        end
+        toggle = onmatch.captures[3]::AbstractString
+        if toggle == "off"
+            @debug "Ignoring `$(onmatch.match)` toggle since formatting is already off."
+            accept_node!(ctx, lkid)
+            continue
+        end
+        @assert toggle == "on"
+        if !validate_toggle(ctx, kids, j)
+            @debug "Ignoring `$(onmatch.match)` toggle since it is not on a separate line."
+            accept_node!(ctx, lkid)
+            continue
+        end
+        seek(ctx.fmt_io, pos)
+        return j
+    end
+    # Reset the stream
+    seek(ctx.fmt_io, pos)
+    # No closing toggle found. This is allowed as a top level statement so that complete
+    # files can be ignored by just a comment at the top.
+    if length(ctx.lineage_kinds) == 1 && ctx.lineage_kinds[1] === K"toplevel"
+        return typemax(Int)
+    end
+    @debug "Ignoring `$(offmatch.match)` toggle since no matching `on` toggle " *
+        "was found at the same tree level."
+    return nothing
+end
+
 function format_node_with_kids!(ctx::Context, node::Node)
     # If the node doesn't have kids there is nothing to do here
     if is_leaf(node)
@@ -219,6 +312,10 @@ function format_node_with_kids!(ctx::Context, node::Node)
     kids′ = kids
     any_kid_changed = false
 
+    # This method should never be called if formatting is off for this node
+    @assert ctx.format_on
+    format_on_idx = typemin(Int)
+
     # Loop over all the kids
     for (i, kid) in pairs(kids)
         # Set the siblings: previous from kids′, next from kids
@@ -227,6 +324,18 @@ function format_node_with_kids!(ctx::Context, node::Node)
         kid′ = kid
         this_kid_changed = false
         itr = 0
+        # Check if this kid toggles formatting off
+        if ctx.format_on && i > format_on_idx
+            format_on_idx′ = check_format_toggle(ctx, node, kid, i)
+            if format_on_idx′ !== nothing
+                ctx.format_on = false
+                format_on_idx = format_on_idx′
+            end
+        elseif !ctx.format_on && i > format_on_idx - 2
+            # The formatter is turned on 2 steps before so that we can format
+            # the indent of the `#! format: on` comment.
+            ctx.format_on = true
+        end
         # Loop until this node reaches a steady state and is accepted
         while true
             # Keep track of the stream position and reset it below if the node is changed
@@ -286,6 +395,11 @@ Format a node. Return values:
  - `node::JuliaSyntax.GreenNode`: The node should be replaced with the new node
 """
 function format_node!(ctx::Context, node::Node)::Union{Node, Nothing, NullNode}
+    # If formatting is off just return
+    if !ctx.format_on
+        accept_node!(ctx, node)
+        return nothing
+    end
     node_kind = kind(node)
 
     # Not that two separate `if`s are used here because a node like `else` can be both


Test patch:
diff --git a/test/runtests.jl b/test/runtests.jl
index 9209deb..24675d9 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -924,6 +924,68 @@ end
     end
 end
 
+@testset "# runic: (on|off)" begin
+    for exc in ("", "!"), word in ("runic", "format")
+        on = "#$(exc) $(word): on"
+        off = "#$(exc) $(word): off"
+        bon = "#$(exc == "" ? "!" : "") $(word): on"
+        # Disable rest of the file from top level comment
+        @test format_string("$off\n1+1") == "$off\n1+1"
+        @test format_string("1+1\n$off\n1+1") == "1 + 1\n$off\n1+1"
+        @test format_string("1+1\n$off\n1+1\n$on\n1+1") == "1 + 1\n$off\n1+1\n$on\n1 + 1"
+        @test format_string("1+1\n$off\n1+1\n$bon\n1+1") == "1 + 1\n$off\n1+1\n$bon\n1+1"
+        # Toggle inside a function
+        @test format_string(
+            """
+            function f()
+                $off
+                1+1
+                $on
+                1+1
+            end
+            """,
+        ) == """
+        function f()
+            $off
+            1+1
+            $on
+            1 + 1
+        end
+        """
+        @test format_string(
+            """
+            function f()
+                $off
+                1+1
+                $bon
+                1+1
+            end
+            """,
+        ) == """
+        function f()
+            $off
+            1 + 1
+            $bon
+            1 + 1
+        end
+        """
+        @test format_string(
+            """
+            function f()
+                $off
+                1+1
+                1+1
+            end
+            """,
+        ) == """
+        function f()
+            $off
+            1 + 1
+            1 + 1
+        end
+        """
+    end
+end
 
 const share_julia = joinpath(Sys.BINDIR, Base.DATAROOTDIR, "julia")
 if Sys.isunix() && isdir(share_julia)