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

Fusion internal formatting #39

Open
Dionysusnu opened this issue Aug 31, 2021 · 10 comments
Open

Fusion internal formatting #39

Dionysusnu opened this issue Aug 31, 2021 · 10 comments
Labels
not ready - evaluating Currently gauging feedback targeting: docs Improvements or additions to documentation

Comments

@Dionysusnu
Copy link
Contributor

Dionysusnu commented Aug 31, 2021

There are currently a few formatting decisions in Fusion, that are not widely used, for a few reasons:

  • No trailing commas in multi-line tables, and no newlines at end of files
    • This hurts commit diffs, because adding new code will modify the previous line
  • Empty blocks or single statement blocks being multi-line
    • Things like if condition then return end and function() end
    • This is usually just as clean when put on single lines, although admittedly writer preference.
  • No whitespace between table brace and items
    • This hurts readability by making tables seem smushed together.
    • This is currently not consistent in the codebase, with only some parts using whitespace.
  • Operators are often placed after their left expressions, when their right expression is on the next line
    • This harms readability and looks confusing.
  • There are unnecessary empty lines at the starts or ends of blocks
    • As the existing style guide says, this only makes the file harder to read.
  • Operators are mixed in whitespace usage, in order to signify precendence
    • Although this helps with detailed looks at math, it harms readability because of the code being too compact.
@dphfox
Copy link
Owner

dphfox commented Aug 31, 2021

This is definitely stuff we need to sort out, so thanks for raising the issue!

Quick comment on:

Empty blocks or single statement blocks being multi-line

I try not to place multiple things on the same line for diffing purposes - imagine we have the following code:

if x < 0 then return end

If we had to add some code to run inside that special case, it wouldn't diff as nicely:

- if x < 0 then return end
+ if x < 0 then
+     doSomething(x)
+     return
+ end

Versus the current standard, which diffs better:

if x < 0 then
    return
end
  if x < 0 then
+     doSomething(x)
      return
  end

By the way, anyone else who has issues with formatting, please drop them in here - we need to review this before we write too much code 🙂

@dphfox dphfox pinned this issue Aug 31, 2021
@dphfox dphfox added targeting: docs Improvements or additions to documentation not ready - evaluating Currently gauging feedback labels Aug 31, 2021
@dphfox
Copy link
Owner

dphfox commented Aug 31, 2021

For reference, the current style guide (needs updating): https://github.com/Elttob/Fusion/blob/main/style-guide.md

@Dionysusnu
Copy link
Contributor Author

I try not to place multiple things on the same line for diffing purposes - imagine we have the following code:

if x < 0 then return end

If we had to add some code to run inside that special case, it wouldn't diff as nicely:

- if x < 0 then return end
+ if x < 0 then
+     doSomething(x)
+     return
+ end

I don't see this bigger diff as a big issue, because the affected lines are still correct. The reason for trailing commas and final newlines are that the diffs would affect unrelated lines. That's not the case here, so I don't think the same reasoning should apply.

@dphfox
Copy link
Owner

dphfox commented Aug 31, 2021

I think it'll be worth waiting to see what other people think then - I remain unconvinced, but I'm open to changing my mind if other people also think differently.

@Anaminus
Copy link
Contributor

For almost all points, there are going to be no strong reasons to pick one way over the other, and trying to enforce a custom format manually is a exercise in futility. I would recommend finding a formatter (StyLua seems good), pick the configuration that you like, and just roll with that. Any variability allowed by the the formatter is fair game.

@CaptinLetus
Copy link

If we strictly use the Roblox Lua guidelines, multiple statements should be on multiple lines https://roblox.github.io/lua-style-guide/#general-punctuation

@autonordev
Copy link

autonordev commented Sep 1, 2021

I would use the Roblox styleguide - it just saves time having this discussion.

Styleguides tend to be more arbitrary than practical. However special consideration should be given for how the diffs and readers are affected.

@noah-peeters
Copy link

Can highly recommend StyLua. It does however automatically create parentheses for constructors like New("Frame" )({}) .

@Dionysusnu
Copy link
Contributor Author

Dionysusnu commented Sep 1, 2021

I would use the Roblox styleguide - it just saves time having this discussion.

Styleguides tend to be more arbitrary than practical. However special consideration should be given for how the diffs and readers are affected.

The Roblox styleguide is quite outdated, and does not cover a range of topics, most notably Luau types.

Can highly recommend StyLua. It does however automatically create parentheses for constructors like New("Frame" )({}) .

This is an ongoing issue in StyLua. JohnnyMorganz/StyLua#242

@dphfox
Copy link
Owner

dphfox commented Mar 1, 2022

Deferring to v0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not ready - evaluating Currently gauging feedback targeting: docs Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

6 participants