`remove_extra_newlines=true` sometimes removes newlines that are not extra

[kleinschmidt]

We have JuliaFormatter (using the built in YAS config) turned on as a linting step in many of our internal repositories and I find that sometimes it appears a bit...overeager in removing "extra" newlines. Unfortunately because these are internal repos I haven't been able to come up with a MWE, nor have I been able to come up with any discernible pattern of when this behavior occurs, but a recent example (not tested!) is like

function my_func(args, blah...)
    args in (:allowed1, :allowed2) ||
        throw(ArgumentError("not allowed! $args"))

    @info "we did it $(args)"

    localthing = args.thing
    stuff = stuff(localthing)

    otherstuff = do_stuff_with_stuff(stuff)
    return otherstuff
end

and formatter removed the last empty line, but left everything else untouched.

I'll try to come up with an MWE but I figured I'd report here anyway even though it's a partial report in case anyone has any idea what's going on!

[domluna]

is this what you mean

function my_func(args, blah...)
    args in (:allowed1, :allowed2) ||
        throw(ArgumentError("not allowed! $args"))

    @info "we did it $(args)"

    localthing = args.thing
    stuff = stuff(localthing)

    otherstuff = do_stuff_with_stuff(stuff)



    return otherstuff

end

julia> print(format_text(s, remove_extra_newlines=true))
function my_func(args, blah...)
    args in (:allowed1, :allowed2) || throw(ArgumentError("not allowed! $args"))

    @info "we did it $(args)"

    localthing = args.thing
    stuff = stuff(localthing)

    otherstuff = do_stuff_with_stuff(stuff)

    return otherstuff
end

as in the last empty line is removed?

https://github.com/domluna/JuliaFormatter.jl/blob/master/src/fst.jl#L608-L611 does this so it's intentional.

[kleinschmidt]

this is what I mean:

https://github.com/beacon-biosignals/Ray.jl/pull/180#discussion_r1344724092

function deserialize_from_ray_object(x::SharedPtr{ray_jll.RayObject},
                                     outer_object_ref=nothing)
    # unlike CoreWorker::GetData, CoreWorker::GetMetadata returns a _reference_
    # to a pointer to a buffer, so we need to dereference the return value to
    # get the pointer that `take!` expects.
    metadata_ptr = ray_jll.GetMetadata(x[])[]
    # the pointer itself will not be null, but rather point to a null ref
    if !isnull(metadata_ptr[])
        metadata_bytes = take!(metadata_ptr)
        if !isempty(metadata_bytes)
            @warn "Unhandled RayObject.Metadata: $(String(metadata_bytes))"
        end
    end
    
    bytes = take!(ray_jll.GetData(x[]))
    s = RaySerializer(IOBuffer(bytes))
    result = try
        deserialize(s)
    catch
        log_deserialize_error(bytes)
        rethrow()
    end

    for inner_object_ref in s.object_refs
        _register_ownership(inner_object_ref, outer_object_ref)
    end

    # TODO: add an option to not rethrow
    # https://github.com/beacon-biosignals/Ray.jl/issues/58
    result isa RayTaskException ? throw(result) : return result
end

the single empty line before bytes = ... gets removed

[kleinschmidt]

aha, I think I miiight see what's goign on here...the "empty line" is not actually empty but actually includes 4 trailing spaces (editor inserted them and I didn't catch when committing). so perhaps formatter is removing the whitespace...and the line is collateral damage

[kleinschmidt]

It does seem to be specific to having remove_extra_newlines enabled, and does not trigger when I've disabled that pass (just removes the extra whitespace)

[kleinschmidt]

actually, scratch that...I'm now having trouble reproducing this locally. I'll keep an eye out for other examples of this behavior though.

[kleinschmidt]

Other things I've ruled out: weird carriage return stuff (nope, just \ns in here)

[domluna]

aha, I think I miiight see what's goign on here...the "empty line" is not actually empty but actually includes 4 trailing spaces (editor inserted them and I didn't catch when committing). so perhaps formatter is removing the whitespace...and the line is collateral damage

that shouldn't matter "\n " would just be simplified to "\n"