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.

Sign up for GitHub

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

Jump to bottom

Add rule for braces and code alignment #2001

Merged
merged 9 commits into from
May 27, 2023
Merged

Add rule for braces and code alignment #2001

merged 9 commits into from
May 27, 2023

Conversation

atulgpt
Copy link
Contributor

@atulgpt atulgpt commented May 7, 2023 •
edited
Loading

Description

Add rule such that code in function block should have new line in between braces and code

Closes #1938

Checklist

  • PR description added
  • tests are added
  • KtLint has been applied on source code itself and violations are fixed
  • documentation is updated
  • CHANGELOG.md is updated

In case of adding a new rule:

paul-dingemans
paul-dingemans requested changes May 8, 2023
View reviewed changes
Copy link
Collaborator

@paul-dingemans paul-dingemans left a comment

Choose a reason for hiding this comment

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

Tnx for your contribution. I can see that you really picked up a lot of conventions that are used in the project. Well done and thanks for that. Most remarks are about little things that you missed. From a consistency perspective, I would like them to be refactored.

Please make sure to run gradle tasks test and ktlintFormat before pushing commits. This will reveal that your rule is not yet added to the StandardRuleSetProvider and fixes formatting errors.

...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEnd.kt Outdated Show resolved Hide resolved
...in/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEndTest.kt Outdated Show resolved Hide resolved
...in/com/pinterest/ktlint/ruleset/standard/rules/NoSameLineStatementAsOfBlockStartOrEndTest.kt Outdated Show resolved Hide resolved
@paul-dingemans paul-dingemans added this to the 1.0 (Yeah!) milestone May 8, 2023
@atulgpt atulgpt marked this pull request as ready for review May 9, 2023 23:22
@atulgpt
Copy link
Contributor Author

atulgpt commented May 10, 2023

Hi, @paul-dingemans I have updated the PR with documentation, TCs with other cases. Please re-review

@paul-dingemans
Merge remote-tracking branch 'origin/master' into pr/2001
67a02a8
@paul-dingemans
Add blank line
79e8014
@paul-dingemans
Move to snapshot documentation
c7b77c5
@paul-dingemans
Refactor
1fb8eed 
  - add newlines
  - use body expression
@paul-dingemans
Refactor
cc07340 
  - Depend on low level element types when possible
  - Make code more robust
  - Make lint message more clear
  - Fix lint offsets for closing brace
  - Make tests more concise
@paul-dingemans paul-dingemans marked this pull request as draft May 14, 2023 18:57
@paul-dingemans
Copy link
Collaborator

I have refactored the rule and test quite a bit. The base however was solid and made it more easy for me to do this. So your effort is still appreciated.

paul-dingemans
paul-dingemans approved these changes May 14, 2023
View reviewed changes
Copy link
Collaborator

@paul-dingemans paul-dingemans left a comment

Choose a reason for hiding this comment

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

Ready for merge in 1.0

@atulgpt
Copy link
Contributor Author

atulgpt commented May 14, 2023

Thanks @paul-dingemans. Let me know if there is more Up for grabs issue

@paul-dingemans paul-dingemans mentioned this pull request May 15, 2023
Closed
@paul-dingemans
Copy link
Collaborator

Thanks @paul-dingemans. Let me know if there is more Up for grabs issue

Would be cool if you want to help out more. It would be best if you connect to me on the kotlinlang slack, or have a look at https://github.com/pinterest/ktlint/issues?q=is%3Aopen+is%3Aissue+label%3Aup-for-grabs.

@paul-dingemans paul-dingemans self-requested a review May 27, 2023 18:21
@paul-dingemans paul-dingemans marked this pull request as ready for review May 27, 2023 18:21
@paul-dingemans paul-dingemans merged commit 3e56387 into pinterest:master May 27, 2023
@atulgpt atulgpt deleted the feat/1938/add-no-statement-start-or-end branch May 27, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paul-dingemans paul-dingemans paul-dingemans approved these changes

Assignees
No one assigned
Labels
feature changelog
Projects
None yet
Milestone
0.50.0
Development

Successfully merging this pull request may close these issues.

Statement may not start or end on same line as start/end of block
2 participants
@atulgpt @paul-dingemans