Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ktfmt removes blank lines between when clauses #342

Closed
ghost opened this issue Jul 23, 2022 · 5 comments
Closed

ktfmt removes blank lines between when clauses #342

ghost opened this issue Jul 23, 2022 · 5 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@ghost
Copy link

ghost commented Jul 23, 2022

Per https://kotlinlang.org/docs/coding-conventions.html#control-flow-statements, "In a when statement, if a branch is more than a single line, consider separating it from adjacent case blocks with a blank line"

Unfortunately, ktfmt removes such blank lines.

fun foo() {
    when {
        1 == 1 -> {
            println("The world is working okay")
        }
       
        1 > 2 -> {
            println("We may be in a parallel universe with different rules, do not proceed")
        }
    }
}

The blank line between the two cases gets removed, reproducible in https://facebookincubator.github.io/ktfmt/

@cgrushko
Copy link
Contributor

Sounds reasonable. Do you want to take a stab at it?

@salvatorebenedetto
Copy link
Contributor

Couple of questions here.

  1. The original question mentions "if a branch is more than a single line", but that's a little ambiguous. Is it referring to multi-statement blocks? Or simply blocks with braces (as in the example)? I would assume the latter.
  2. It's also not clear how to handle this. Do we insert a blank line between blocks no matter what? Or only if the user started with a blank line? For the sake of consistency, I would recommend always to insert a blank line.

@ghost
Copy link
Author

ghost commented Oct 27, 2022

I guess my preference would be to preserve existing blank lines, but always inserting one also seems reasonable to me.

@juliatuttle
Copy link

juliatuttle commented Oct 27, 2023

I'd like to second this request. In code like this:

when {
    // Comment A
    condition1 -> false

    // Comment B
    condition2 ||
        condition3 ||
        condition4 -> false

    else -> true
}

...I'd like ktfmt to preserve the newline before else, so it doesn't get lumped under Comment B when it's unrelated.

Perhaps this should be a separate issue -- preserving blank lines in when clauses, versus enforcing them between two branches when either branch spans multiple lines.

@hick209 hick209 added good first issue Good for newcomers enhancement New feature or request labels Mar 26, 2024
facebook-github-bot pushed a commit that referenced this issue Jun 7, 2024
Summary:
Implemented as requested on
#342

Reviewed By: davidtorosyan

Differential Revision: D58271914

fbshipit-source-id: 048d9a754b40200f313654775eb743c024073d39
@hick209
Copy link
Contributor

hick209 commented Jun 7, 2024

Fixed with 7d23e59

@hick209 hick209 closed this as completed Jun 7, 2024
ZacSweers referenced this issue in ZacSweers/CatchUp Jun 14, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.facebook:ktfmt](https://github.com/facebook/ktfmt) | `0.49` ->
`0.51` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/com.facebook:ktfmt/0.51?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.facebook:ktfmt/0.51?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.facebook:ktfmt/0.49/0.51?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.facebook:ktfmt/0.49/0.51?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>facebook/ktfmt (com.facebook:ktfmt)</summary>

###
[`v0.51`](https://github.com/facebook/ktfmt/blob/HEAD/CHANGELOG.md#051)

##### Added

-   Created CHANGELOG.md
- Added --help option to CLI
([https://github.com/facebook/ktfmt/pull/477](https://github.com/facebook/ktfmt/pull/477))

##### Changed

- Preserves blank spaces between when clauses
([https://github.com/facebook/ktfmt/issues/342](https://github.com/facebook/ktfmt/issues/342))
-   Named the default style as `Formatter.META_FORMAT` / `--meta-style`
-   `FormattingOptions` constructor parameters order was changed

##### Fixed

- Compilation issues with online formatter
(facebook/ktfmt@8605080)
- Removing valid semicolons
([https://github.com/facebook/ktfmt/issues/459](https://github.com/facebook/ktfmt/issues/459))
- Incorrect detection of unused `assign` import
([https://github.com/facebook/ktfmt/issues/411](https://github.com/facebook/ktfmt/issues/411))

##### Removed

- **Deleted `Formatter.DROPBOX_FORMAT` / `--dropbox-style` (BREAKING
CHANGE)**
-   Deleted `FormattingOptions.Style` enum

### [`v0.50`](https://github.com/facebook/ktfmt/releases/tag/v0.50):
0.50

#### Changelog

- Add pre commit hooks to readme
([https://github.com/facebook/ktfmt/pull/462](https://github.com/facebook/ktfmt/pull/462))
– [@&#8203;0x26res](https://github.com/0x26res)
- Add homebrew installation note to readme
([https://github.com/facebook/ktfmt/pull/468](https://github.com/facebook/ktfmt/pull/468))
– [@&#8203;chenrui333](https://github.com/chenrui333)
- Refactor CLI argument parsing
([https://github.com/facebook/ktfmt/pull/467](https://github.com/facebook/ktfmt/pull/467))
– [@&#8203;grodin](https://github.com/grodin)
- Fix issue with context receive in lambdas
([https://github.com/facebook/ktfmt/issues/471](https://github.com/facebook/ktfmt/issues/471))
– [@&#8203;hick209](https://github.com/hick209)
- Don't reorder [@&#8203;sample](https://github.com/sample) tag
([https://github.com/facebook/ktfmt/issues/406](https://github.com/facebook/ktfmt/issues/406))
– [@&#8203;davidtorosyan](https://github.com/davidtorosyan)

**Full Changelog**:
facebook/ktfmt@v0.49...v0.50

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/ZacSweers/CatchUp).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
slack-oss-bot referenced this issue in slackhq/foundry Jun 16, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [com.facebook:ktfmt](https://github.com/facebookincubator/ktfmt) |
dependencies | minor | `0.50` -> `0.51` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>facebookincubator/ktfmt (com.facebook:ktfmt)</summary>

###
[`v0.51`](https://github.com/facebookincubator/ktfmt/blob/HEAD/CHANGELOG.md#051)

##### Added

-   Created CHANGELOG.md
- Added --help option to CLI
([https://github.com/facebook/ktfmt/pull/477](https://github.com/facebook/ktfmt/pull/477))

##### Changed

- Preserves blank spaces between when clauses
([https://github.com/facebook/ktfmt/issues/342](https://github.com/facebook/ktfmt/issues/342))
-   Named the default style as `Formatter.META_FORMAT` / `--meta-style`
-   `FormattingOptions` constructor parameters order was changed

##### Fixed

- Compilation issues with online formatter
(facebook/ktfmt@8605080)
- Removing valid semicolons
([https://github.com/facebook/ktfmt/issues/459](https://github.com/facebook/ktfmt/issues/459))
- Incorrect detection of unused `assign` import
([https://github.com/facebook/ktfmt/issues/411](https://github.com/facebook/ktfmt/issues/411))

##### Removed

- **Deleted `Formatter.DROPBOX_FORMAT` / `--dropbox-style` (BREAKING
CHANGE)**
-   Deleted `FormattingOptions.Style` enum

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MDguMiIsInVwZGF0ZWRJblZlciI6IjM3LjQwOC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
slack-oss-bot referenced this issue in slackhq/circuit Jun 16, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [com.facebook:ktfmt](https://github.com/facebook/ktfmt) |
dependencies | minor | `0.50` -> `0.51` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>facebook/ktfmt (com.facebook:ktfmt)</summary>

###
[`v0.51`](https://github.com/facebook/ktfmt/blob/HEAD/CHANGELOG.md#051)

##### Added

-   Created CHANGELOG.md
- Added --help option to CLI
([https://github.com/facebook/ktfmt/pull/477](https://github.com/facebook/ktfmt/pull/477))

##### Changed

- Preserves blank spaces between when clauses
([https://github.com/facebook/ktfmt/issues/342](https://github.com/facebook/ktfmt/issues/342))
-   Named the default style as `Formatter.META_FORMAT` / `--meta-style`
-   `FormattingOptions` constructor parameters order was changed

##### Fixed

- Compilation issues with online formatter
(facebook/ktfmt@8605080)
- Removing valid semicolons
([https://github.com/facebook/ktfmt/issues/459](https://github.com/facebook/ktfmt/issues/459))
- Incorrect detection of unused `assign` import
([https://github.com/facebook/ktfmt/issues/411](https://github.com/facebook/ktfmt/issues/411))

##### Removed

- **Deleted `Formatter.DROPBOX_FORMAT` / `--dropbox-style` (BREAKING
CHANGE)**
-   Deleted `FormattingOptions.Style` enum

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MDguMiIsInVwZGF0ZWRJblZlciI6IjM3LjQwOC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants