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

Add blank lines after semicolons #33

Merged
merged 4 commits into from
Sep 16, 2021

Conversation

macintoshpie
Copy link
Contributor

@macintoshpie macintoshpie commented Aug 20, 2021

background

The formatter currently doesn't add any empty lines between code making it less readable (see #32 )

changes

  • insert a blank line after every semicolon, with some exceptions (within statement, inside [] brackets)

Thanks to @trivvz for doing most of the work!

tpwo and others added 3 commits August 16, 2021 23:14
Vertical spacing makes the code more legible. This version adds a single
blank line after almost every semicolon.

There are two exceptions:
- If there`s a `within` on a given line, a blank line won't be added.
- If inside matrix (i.e. `[]`), a blank line won't be added.

  Example output:

    parameter Integer[2,2] A = [
      1,2;
      3,4];

Also:
- Add example with formatting arrays and matrices

Known issues:
- The formatted file will have two blank lines at the end. The solution
  would be to detect if we're on line with the last `end` within the
  file, and then skip adding an additional blank line. There might be
  multiple `end` in a file, so this is not that obvious to solve.
We need to find a way to get rid of duplicated blank line at the end of
files that have semicolon in the last line.
Necessary in order to prevent extra newlines at the end of files
Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks dandy to me! ⚡️

@macintoshpie
Copy link
Contributor Author

@mwetter @AntoineGautier any reservations about this change?

@mwetter
Copy link

mwetter commented Aug 20, 2021

Can we suppress these empty lines in the equation section (and the initial equation, algorithm and initial algorithm) section?
I think these sections are well readable in editors that have syntax coloring, and having all these spaces just takes away what can be seen on one screen.
In the declaration section (above the initial equation section), I am fine with the change, although for parameter and constant declarations I would prefer not adding spaces in between, because these are often only one or two lines long.
Having spaces between the generally longer instantiation clauses of objects make the file more readable.

This probably also addresses item 2 in #26

@tpwo
Copy link
Contributor

tpwo commented Aug 20, 2021

@mwetter, yeah, I agree that adding blank lines between all single line statements is a bit of space waste.

I think that the ideal approach would be to allow both for zero and single blank line space between one-liners (sometimes it's good to split some groups of single-line equations etc from each other). But that means reading this information during parsing and saving it for later use.

A simpler solution for now: maybe just insert blank lines only before and after multiline statements no matter where they are?

With the current formatting output, parameters/constants with annotations are also multiline, though. But TBH, I would still prefer a blank line between them in this situation.

Regarding the implementation, I see that it would still be possible to see a blank line after singleline statement if the next one is multiline:

singleline;
singleline;

multiline
multiline;

multiline
multiline
multiline;

singleline;

@mwetter
Copy link

mwetter commented Aug 20, 2021

@trivvz : I looked at a few files and think empty lines are most useful if the code switches between sections that have different declarations, e.g., after all parameters, before protected, before equation, before // (comments), etc.

Looking at files such as https://github.com/lbl-srg/modelica-buildings/blob/master/Buildings/BoundaryConditions/SolarIrradiation/DiffusePerez.mo
I think syntax coloring makes them good readable without these extra empty lines, and adding many empty lines just reduces what one can see on the screen.

@macintoshpie
Copy link
Contributor Author

Hmm, maybe just make this optional for now as a "beta" feature until we find a more generally satisfying solution. I can get those changes up soon.

Regarding a more general fix, maybe we should tokenize empty lines and store them like we do comments, then use that to determine if/where to insert empty lines - allowing users to "keep" their lines if they want to. Maybe do something like black's rules around empty lines: https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#empty-lines

@macintoshpie macintoshpie merged commit 366e8bb into develop Sep 16, 2021
@macintoshpie macintoshpie deleted the refactor/insert-blank-line-edits branch September 16, 2021 20:49
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.

4 participants