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

ambiguous linebreaking of assignments (question) #98

Closed
exaexa opened this issue Nov 10, 2024 · 7 comments
Closed

ambiguous linebreaking of assignments (question) #98

exaexa opened this issue Nov 10, 2024 · 7 comments

Comments

@exaexa
Copy link

exaexa commented Nov 10, 2024

Hello!

is Runic formatting output supposed to depend on changes of blank characters in the input?

In particular I found a few spots where Runic accepts two different line-breakings as "OK" and doesn't change them (i.e., does not enforce the unique formatting), no matter that they are equivalent as source code:

x = 0

x =
    0

Is this intended? If it is, is there be some way (option perhaps) to enforce a completely blanks-insensitive formatting?

Thanks!
-mk

cc @stelmo

@exaexa
Copy link
Author

exaexa commented Nov 10, 2024

as a note, the "line-breaked" form indentation looks particularly sub-optimal in cases; e.g. here:

    line_system =
        :point^C.ConstraintTree(
        :x => C.Constraint(0 + 2 * line_param),
        :y => C.Constraint(0 + 1 * line_param),
    )

(taken from ConstraintTrees.jl)

@exaexa
Copy link
Author

exaexa commented Nov 10, 2024

another example (again taken plain from ConstraintTrees:)

Base.:+(a::LinearValue, b::LinearValue) =
let
    (idxs, weights) =
        add_sparse_linear_combination(a.idxs, a.weights, b.idxs, b.weights)
    LinearValue(; idxs, weights)
end

#vs.

Base.:+(a::LinearValue, b::LinearValue) = let
    (idxs, weights) = add_sparse_linear_combination(a.idxs, a.weights, b.idxs, b.weights)
    LinearValue(; idxs, weights)
end

@fredrikekre
Copy link
Owner

Hi, in general Runic doesn't modify newlines in the source. So for example, if you have put a line break after = it will still be there.

x = 0

x =
    0

as a note, the "line-breaked" form indentation looks particularly sub-optimal in cases; e.g. here [...]

Yea but doesn't it almost always look better with no newline after the =? Like

line_system = :point^C.ConstraintTree(
    :x => C.Constraint(0 + 2 * line_param),
    :y => C.Constraint(0 + 1 * line_param),
)
Base.:+(a::LinearValue, b::LinearValue) =
let
    (idxs, weights) =
        add_sparse_linear_combination(a.idxs, a.weights, b.idxs, b.weights)
    LinearValue(; idxs, weights)
end

This would again look better as function ... end or at least with the let on the same line as =. This particular case was discussed a bit in #39

@exaexa
Copy link
Author

exaexa commented Nov 11, 2024

@fredrikekre ah I see, thanks for the explanation.

Just a few notes about the examples:

line_system =
    :point^C.ConstraintTree(
    :x => C.Constraint(0 + 2 * line_param),
    :y => C.Constraint(0 + 1 * line_param),
)

this is mildly confusing because the "point", "x" and "y" look like the same-level kind of thing, because they "share the indent"

Re the second example, full agree that it would be better with function but we got a relatively good form from JuliaFormatter (as you said, with = on the same line) so was wondering what went wrong.

The property with retained linebreaks explains it, this issue is then effectively not an issue, so closing. Thanks again! :)

@exaexa exaexa closed this as completed Nov 11, 2024
@fredrikekre
Copy link
Owner

this is mildly confusing because the "point", "x" and "y" look like the same-level kind of thing,

I agree, it is tricky. This is related to the "hard" vs "soft" indenting discussed in https://github.com/fredrikekre/Runic.jl/blob/master/README.md#indentation.

Going with just "hard" indent would do

line_system =
    :point^C.ConstraintTree(
        :x => C.Constraint(0 + 2 * line_param),
        :y => C.Constraint(0 + 1 * line_param),
    )

becuase the =-expression and the call to ConstraintTree are multiline expressions. However, if you didn't have the newline after = it would become

line_system = :point^C.ConstraintTree(
    :x => C.Constraint(0 + 2 * line_param),
    :y => C.Constraint(0 + 1 * line_param),
)

so this is also a bit inconsistent.

@exaexa
Copy link
Author

exaexa commented Nov 11, 2024

Yeah, the hard indent might be too much. The "slightly inconsistent" is mildly inconsistent w.r.t. AST (depending on how you define consistency) but at least consistent w.r.t. user-visible brackets (parentheses in this case).

Just to be complete about the design space, this is a possible middle ground taken by many formatters:

line_system = :point^C.ConstraintTree(
        :x => C.Constraint(0 + 2 * line_param),
        :y => C.Constraint(0 + 1 * line_param),
    )

@fredrikekre
Copy link
Owner

Yea, I don't mind that formatting, just isn't how most existing (Julia) code look like. For example, if I have code like this:

push!(
    x, 
    1,
    2
)

and then later want to assign the result of this call and adding y = in front of the push!. With the existing Runic implementation there wouldn't be any extra diff generated from indenting all the following lines too.

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

No branches or pull requests

2 participants