facebook / ktfmt

A program that reformats Kotlin source code to comply with the common community standard for Kotlin code conventions.
https://facebook.github.io/ktfmt/
Apache License 2.0
933 stars 78 forks source link

Support disabling ordering doc tags in command line #474

Closed omarismail94 closed 5 months ago

omarismail94 commented 5 months ago

Tested: Added unit test

Also compiled the JAR and tested it running:

 java -jar ./ktfmt-0.50-SNAPSHOT-jar-with-dependencies.jar --kotlinlang-style --do-not-order-doc-tags compose/runtime/runtime-saveable/src/commonMain/kotlin/androidx/compose/runtime/saveable/RememberSaveable.kt

and it did not re-order the kdcoc tags in the kotlin file, as expected

Fixed: Issue #406

facebook-github-bot commented 5 months ago

Hi @omarismail94!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

omarismail94 commented 5 months ago

Signed CLA

facebook-github-bot commented 5 months ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

facebook-github-bot commented 5 months ago

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

hick209 commented 5 months ago

Is it necessary to expose the orderDocTag as a CLI option? This would go against this principle.

What about just setting it as part of the Google Style?

omarismail94 commented 5 months ago

Thanks! Please see https://github.com/facebook/ktfmt/issues/406 on why we want to expose this

hick209 commented 5 months ago

I understand why the feature is needed, but not why you need direct CLI exposure yet. A workaround for it would be to build a simple Java program and runs ktfmt for you, where you set it up appropriately, if you don't want to touch the Google style

omarismail94 commented 5 months ago

I understand why the feature is needed, but not why you need direct CLI exposure yet. A workaround for it would be to build a simple Java program and runs ktfmt for you, where you set it up appropriately, if you don't want to touch the Google style

Ah OK, so I can keep the feature exposed in FormattingOptions, but not via CLI?

I wanted to expose in CLI as that way, if I were to package the JAR as in intellij plugin, I can give it the flag to not re-order the tags. This would be a nice to have.

omarismail94 commented 5 months ago

I changed it so orderDocTags is only in FormattingOptions and not avaialble via CLI

omarismail94 commented 5 months ago

Im going to close this in preference for PR #475