fredrikekre / Runic.jl

A code formatter for Julia with rules set in stone.
MIT License
111 stars 4 forks source link

#! format off does not completely disable formatting #107

Closed j-fu closed 1 day ago

j-fu commented 4 days ago

Hi, thanks for runic! I am in the process of being persuaded to agree that explicit return statements are a good thing for those reading my code. However:

Every time Pluto saves a notebook to disk, it writes the following statement to the notebook:

macro bind(def, element)
    #! format: off
    quote
        local iv = try Base.loaded_modules[Base.PkgId(Base.UUID("6e696c72-6542-2067-7265-42206c756150"), "AbstractPlutoDingetjes")].Bonds.initial_value catch; b -> missing; end
        local el = $(esc(element))
        global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : iv(el)
        el
    end
    #! format: on
end

Runic wants to format this as

macro bind(def, element)
    #! format: off
    return quote
        local iv = try Base.loaded_modules[Base.PkgId(Base.UUID("6e696c72-6542-2067-7265-42206c756150"), "AbstractPlutoDingetjes")].Bonds.initial_value catch; b -> missing; end
        local el = $(esc(element))
        global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : iv(el)
        el
    end
    #! format: on
end

While this might be changed to more "runic" code on the Pluto side (@fonsp), in fact, runic fails the assumption that code in the region marked by formt on/off is not modified. So I guess this is a bug.

fredrikekre commented 3 days ago

Not sure I would call it a bug, but perhaps a result of how things are implemented. For the "add explicit return" formatting pass the syntax node of interest is the function node` (i.e. the whole function) and that one isn't within the toggle comments. Do you have control over this? In that case, using

#! format: off
macro bind(def, element)
    quote
        local iv = try Base.loaded_modules[Base.PkgId(Base.UUID("6e696c72-6542-2067-7265-42206c756150"), "AbstractPlutoDingetjes")].Bonds.initial_value catch; b -> missing; end
        local el = $(esc(element))
        global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : iv(el)
        el
    end
end
#! format: on

would work as you expect.

j-fu commented 3 days ago

Ok, so let us see if this will be changed in Pluto then.

j-fu commented 1 day ago

Will be addressed in Pluto: https://github.com/fonsp/Pluto.jl/pull/3106#issuecomment-2498438716