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

Change indentation from 3 to 2 using fprettify. #81

Merged
merged 4 commits into from Mar 27, 2024

Conversation

semi-h
Copy link
Member

@semi-h semi-h commented Mar 26, 2024

Just wanted to see how fprettify would handle what we want. Only changed the indentation in our config file and run the fprettify in place. I think fprettify gets very annoyed when line length is > 79, and results in a funny indentation.

And fprettify doesn't touch comments starting with !!, so almost all of them are wrongly indented.

@semi-h
Copy link
Member Author

semi-h commented Mar 27, 2024

I did some further tests and with the last 2 commits in the PR, running fprettify in the whole codebase doesn't introduce any errors or changes, meaning that the current state is fully compatible with our fprettify settings. Also, I wasn't able to observe any buggy indentation anywhere in the codebase in its current state. To get here:

  • I had to add use the deactivation string !& in every single CUDA kernel call to disable fprettify for that particular line.
  • I had to manually edit ~20 statements (in total in the whole codebase) and break them into multiple lines so that they don't go beyond 79 characters. I think most bugs we have observed so far were a result of character limit. fprettify doesn't break the line for the user, only warns the user giving exact location, so actually it was easy to target all these statements. And annoying thing is, fprettify warns the user when the line length is too much, but also changes the indentation in a funny way too. But if the line is within the limit, I haven't really noticed any bug.

Although I haven't observed any buggy indentation in the current state, there are two things we need to be aware of

  • We use !! to add in-code documentation via ford, and fprettify doesn't touch any line that starts with !!. This means user is fully responsible setting the correct indentation for these lines.
  • As we know, fprettify is not compatible with CUDA chevron syntax. Currently, I had to use the deactivation string !& in every single kernel call, so that fprettify is deactivated and the style/indentation on that particular line is ignored. This is not ideal but only way if we want to have fprettify doing checks/changes automatically in a reliable way.

@Nanoseb
Copy link
Collaborator

Nanoseb commented Mar 27, 2024

We probably want to update the documentation/coding style to mention the !& syntax and the change to 2 spaces instead of 3

@semi-h semi-h marked this pull request as ready for review March 27, 2024 13:26
@semi-h
Copy link
Member Author

semi-h commented Mar 27, 2024

Added !& to the documentation. For the style preferences documentation refers to .fprettify.ini in the root folder, doesn't explicitly state the indentation level.

@semi-h semi-h merged commit ce2efbc into xcompact3d:main Mar 27, 2024
2 checks passed
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.

None yet

2 participants