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

Rework indentation after assignment #42

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Rework indentation after assignment #42

merged 1 commit into from
Aug 16, 2024

Conversation

fredrikekre
Copy link
Owner

This patch reworks the indentation after assignment. For "blocklike" (try/if/triple strings) right hand sides the indentation is disabled. Superfluous newlines are removed. For example

x =
if a
    x
else
    y
end

will become

x = if a
    x
else
    y
end

A valid alternative is

x = if a
        x
    else
        y
    end

This highlights the start/end of the right hand side of the assignment in a better way, but it doesn't look very nice and this style is never really seen in the wild. For triple strings I think it is OK though, e.g.

x = """
    foo
    bar
    """

vs the current (and seen more in the wild)

x = """
foo
bar
"""

Closes #39.

@adienes
Copy link

adienes commented Aug 12, 2024

so triple quoted strings become

x = """
    foo
    bar
    """

? IMO that is the right choice, just confirming

@fredrikekre
Copy link
Owner Author

This PR doesn't do that, but I have considered that and pushed the code for it to fe/continue-triple-strings some time ago.

I was just getting at the similarity between if ... end and """ ... """. After this PR they are consistent so that they are not indented. If triple strings are indented it would not match the if ... end case, but perhaps it doesn't have to.

@lmshk
Copy link

lmshk commented Aug 13, 2024

Superfluous newlines are removed.

So this will not allow me to break after averyverylongidentifier = for long conditions to avoid crossing a line length limit? Any reason why

averyverylongidentifier =
    if averyverylongcondition && maybeanotherone
        x
    else
        y
    end

is not allowed (if I manually break)?

Would I then need to write

averyverylongidentifier = \
    if averyverylongcondition && maybeanotherone
        x
    else
        y
    end

or

averyverylongidentifier = if (
    averyverylongcondition && maybeanotherone
)
    x
else
    y
end

to achieve this?

@adienes
Copy link

adienes commented Aug 13, 2024

the extremely popular black formatter for Python would do it your third way (kinda, although obviously var = if ... is not valid in Python)

averyverylongidentifier = if (
    averyverylongcondition && maybeanotherone
)
    x
else
    y
end

if averyverylongcondition is long enough, maybe it itself should be broken up over multiple lines? although I know Runic doesn't enforce line limits so this would have to be done by the user

averyverylongidentifier = if (
    half_of_the_long_condition
    || the_other_half_of_condition
    ) && maybe_another_long_condition

    x
else
    y
end

@fredrikekre
Copy link
Owner Author

Perhaps the line break could be left to the user. Runic doesn't really care about line breaks elsewhere.

@lmshk
Copy link

lmshk commented Aug 14, 2024

👍

And if linebreaks are kept, I think

averyverylongidentifier =
    if averyverylongcondition && maybeanotherone
        x
    else
        y
    end

would be the simplest choice ("add 1 indentation level because the line is continued"). If the condition becomes long enough, (/) can still be added manually (then potentially indenting even further inside), and this doesn't preclude the

x = if (
    longcondition
)
   yes
else
   no
end

form either.

@fredrikekre
Copy link
Owner Author

I don't want to indent the block depending on whether there is a preceeding newline or not. It would be

averyverylongidentifier =
if averyverylongcondition && maybeanotherone
    x
else
    y
end

if there is a newline after the =, and

averyverylongidentifier = if averyverylongcondition && maybeanotherone
    x
else
    y
end

if there is not.

I don't think I have seen intended blocks like that in the wild.

Same for """, they seem to always be wrriten like this in the wild

x = """
fdsfs
"""

But for this I am more tempted to indent (compared to if/try blocks).

@lmshk
Copy link

lmshk commented Aug 15, 2024

This confuses me. Currently, Runic also does

 a =
-b
+    b
 
 a =
     b

and further

 a =
-if x; y else z end
+    if x; y else z end

so why should the block form be special-cased? Isn't this just an instance of indenting continued lines like

 a = b +
-c
+    c

?

@fredrikekre
Copy link
Owner Author

I mean, perhaps newline after = just shouldn't be allowed then?

and further [...]

Well, with this PR that would become

a = if x; y else z end

but I think perhaps blocks should be detangled into multiple lines like

a = if x
    y
else
    z
end

fredrikekre added a commit that referenced this pull request Aug 15, 2024
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 #42.
@lmshk
Copy link

lmshk commented Aug 15, 2024

I still don't understand what the problem would be with the

a =
    if x
        y
    else
        z
    end

variant so much that Runic should start removing manual linebreaks for this. I find it perfectly legible and also consistent with the "continued lines add indentation" rule.

But as I wrote elsewhere, that is fine.

(Anyway, I can always just wrap the condition, or the whole expression, in parentheses.)

This patch reworks the indentation after assignment. For "blocklike"
(try/if/triple-strings) right hand sides the indentation is disabled.

Closes #39.
@fredrikekre
Copy link
Owner Author

After some great discussion on Slack it seems that the majority thinks that block constructs like if/try on the right hand side of an assignment should not be indented, but everything else should.

I have updated the PR to do this, and also updated so that newlines between = and the right hand side are kept as in the source. Might want to revisit this later to remove these types of newlines (but they are currently allowed in all other contexts so doesn't make sense to do in the PR anyway).

@fredrikekre fredrikekre merged commit c30af80 into master Aug 16, 2024
14 checks passed
@fredrikekre fredrikekre deleted the fe/assignment branch August 16, 2024 08:16
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.

After linebreaks following =, indentation is off
3 participants