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

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://togithub.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://togithub.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://togithub.com/facebook/ktfmt/pull/477))

##### Changed

- Preserves blank spaces between when clauses
([https://github.com/facebook/ktfmt/issues/342](https://togithub.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://togithub.com/facebook/ktfmt/issues/459))
- Incorrect detection of unused `assign` import
([https://github.com/facebook/ktfmt/issues/411](https://togithub.com/facebook/ktfmt/issues/411))

##### Removed

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

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

#### Changelog

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

##### Changed

- Preserves blank spaces between when clauses
([https://github.com/facebook/ktfmt/issues/342](https://togithub.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://togithub.com/facebook/ktfmt/issues/459))
- Incorrect detection of unused `assign` import
([https://github.com/facebook/ktfmt/issues/411](https://togithub.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://togithub.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://togithub.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://togithub.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://togithub.com/facebook/ktfmt/pull/477))

##### Changed

- Preserves blank spaces between when clauses
([https://github.com/facebook/ktfmt/issues/342](https://togithub.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://togithub.com/facebook/ktfmt/issues/459))
- Incorrect detection of unused `assign` import
([https://github.com/facebook/ktfmt/issues/411](https://togithub.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://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MDguMiIsInVwZGF0ZWRJblZlciI6IjM3LjQwOC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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