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

Convert IntelliJ plugin to Kotlin #502

Closed
wants to merge 11 commits into from

Conversation

rock3r
Copy link
Contributor

@rock3r rock3r commented Aug 7, 2024

Note

This requires #501 to be merged first

This PR is the basis for future improvements of the plugin. In this PR, the plugin code is converted to Kotlin, the configurable is recreated with the Kotlin UI DSL, and the goal is to maintain the existing behaviours.

The only small change is that the style dropdown in the configurable is disabled when ktfmt is disabled.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 7, 2024
Copy link
Contributor

@hick209 hick209 left a comment

Choose a reason for hiding this comment

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

Overall this LGTM, but please mark the files as a move so we don't lose blame history.

@rock3r
Copy link
Contributor Author

rock3r commented Aug 7, 2024

Not sure how to do that? I didn't know you could override what diff thinks happened

@rock3r
Copy link
Contributor Author

rock3r commented Aug 7, 2024

@hick209 please advise so I can look into it tomorrow

@rock3r rock3r force-pushed the convert-ij-plugin-to-kotlin branch from f140876 to e3a78b7 Compare August 8, 2024 10:25
@rock3r
Copy link
Contributor Author

rock3r commented Aug 8, 2024

@hick209 I tried looking into this but I did not find a way to force Git to consider files as moved. I had kept the actual conversion of the files to Kotlin to a dedicated commit, too, but that did not help. Also tried re-doing the conversion in another branch by committing the moving first, and the conversion second, but the conversion still results in pairs of adding/deleting

@hick209
Copy link
Contributor

hick209 commented Aug 8, 2024

Yeah I looked it up as well and noticed this is not really doable in git :-/

I'm used to working with Mercurial, which allows this with the following command:

hg mv FROM TO --after

No worries, I can try to do that internally once we pull your changes. I have to wait until we merge the other 2 PRs for this though.

Thanks a ton for you contributions!

@rock3r rock3r force-pushed the convert-ij-plugin-to-kotlin branch from e3a78b7 to ade7a12 Compare August 8, 2024 10:53
@rock3r rock3r force-pushed the convert-ij-plugin-to-kotlin branch from a652a5e to b56bf5b Compare August 8, 2024 14:49
@facebook-github-bot
Copy link
Contributor

@hick209 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@hick209 merged this pull request in 5d876ca.

@rock3r rock3r deleted the convert-ij-plugin-to-kotlin branch August 12, 2024 15:18
facebook-github-bot pushed a commit that referenced this pull request Aug 14, 2024
Summary:
[This PR](#502) converted the code to Kotlin, but we forgot to adapt our internal buck compilation to it and as a result things are failing.

```
BUILD FAILED
File not found: `fbsource//xplat/ktfmt/ktfmt_idea_plugin/src/main/java/com/facebook/ktfmt/intellij/Notifications.java`
```

Reviewed By: cortinico

Differential Revision: D61208006

fbshipit-source-id: c90f8de7920ab86cda279b07daf7a59b7777a8c2
github-merge-queue bot referenced this pull request in slackhq/foundry Aug 28, 2024
This PR contains the following updates:

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

---

> [!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.52`](https://togithub.com/facebookincubator/ktfmt/blob/HEAD/CHANGELOG.md#052)

##### Fixed

- IntelliJ plugin crash
([https://github.com/facebook/ktfmt/pull/501](https://togithub.com/facebook/ktfmt/pull/501))
- Ordering of `@property` and `@param` in KDoc
([https://github.com/facebook/ktfmt/pull/498](https://togithub.com/facebook/ktfmt/pull/498))
- Annotation in return expressions
([https://github.com/facebook/ktfmt/issues/497](https://togithub.com/facebook/ktfmt/issues/497))

##### Changed

- KotlinLang style also managing trailing commas
([https://github.com/facebook/ktfmt/issues/216](https://togithub.com/facebook/ktfmt/issues/216),
[https://github.com/facebook/ktfmt/issues/442](https://togithub.com/facebook/ktfmt/issues/442))
- Converted IntelliJ plugin to Kotlin
([https://github.com/facebook/ktfmt/pull/502](https://togithub.com/facebook/ktfmt/pull/502))

##### Added

- More stability tests
([https://github.com/facebook/ktfmt/pull/488](https://togithub.com/facebook/ktfmt/pull/488))
- Custom profile in plugin settings, mirroring Gradle/Maven plugins
([https://github.com/facebook/ktfmt/pull/503](https://togithub.com/facebook/ktfmt/pull/503))

</details>

---

### Configuration

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

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, 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:eyJjcmVhdGVkSW5WZXIiOiIzOC40NS4wIiwidXBkYXRlZEluVmVyIjoiMzguNTIuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

---------

Co-authored-by: Zac Sweers <pandanomic@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants