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

Add option to IDEA plugin (and CLI) to format JavaDoc #724 #986

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

blutorange
Copy link

@blutorange blutorange commented Jan 21, 2024

As discussed in #724 (as I understand it), I added an option to the IDEA plugin to enable formattting JavaDoc comments.

  • This adds an option to the IDEA plugin (checkbox) to format JavaDoc comments. It is disabled by default.
  • As the IDEA plugin (sometimes) uses the bootstrapping formatter service, which in turn uses the command line tool, this also add an option --format-javadoc to the command line tool.
  • To pass the options to the FormatterService loaded via service provider, I added a method withOptions to the service provided interface, which returns a new (immutable) instance with the updated options. Compared with the suggestion from the issue comment (Please expose option to format JavaDoc #724 (comment)), this (a) does not require deprecating methods and (b) the options can be set independently of where the formatter is actually used.
  • Since the infrastructure to pass options to the formatter now exists, we could also add more options to the code style select configuration in the IDEA plugin, but I'm not sure if that makes sense (I want to use Palantir, after all).
  • The issue also mentioned API compatibility. The service provider interface is compatible, most other classes and methods are internal. But if somebody more familar with the code base could take another look, that would be great.
  • I also added a few tests for the new option.

Possible downsides?

  • This adds an option that affects the formatter output, and Palantir strives to be an opinionated formatter without any configuration. As discussed in the issue, though, adding an option for fomatting JavaDoc should be ok.

image

* This adds an option to the IDEA plugin (checkbox) to format JavaDoc comments.
  It is disabled by default.
* As the IDEA plugin (sometimes) uses the bootstrapping formatter service, which in turn
  uses the command line tool, this also add an option `--format-javadoc` to
  the command line tool.
* To pass the options to the FormatterService loaded via service provider, I added a
  method `withOptions` to the service provided interface, which returns a new (immutable)
  instance with the updated options. Compared with the suggestion from the issue comment
  (palantir#724 (comment)),
  this (a) does not require deprecating methods and (b) the options can be set independently
  of where the formatter is actually used.
* Since the infrastructure to pass options to the formatter now exists, we could also
  add more options to the code style select configuration in the IDEA plugin, but I'm
  not sure if that makes sense (I want to use Palantir, after all).
* The issue also mentioned API compatibility. The service provider interface is compatible,
  most other classes and methods are internal. But if somebody more familar with the code
  base could take another look, that would be great.
* I also added a few tests for the new option.
@palantirtech
Copy link
Member

Thanks for your interest in palantir/palantir-java-format, @blutorange! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@changelog-app
Copy link

changelog-app bot commented Jan 21, 2024

Generate changelog in changelog/@unreleased

What do the change types mean?
  • feature: A new feature of the service.
  • improvement: An incremental improvement in the functionality or operation of the service.
  • fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
  • break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
    and external consumers of the service's API (e.g. customer-written software or integrations).
  • deprecation: Advertises the intention to remove service functionality without any change to the
    operation of the service itself.
  • manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
    performing database surgery, ...) at the time of upgrade for it to succeed.
  • migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?
  • ❗The break and manual task changelog types will result in a major release!
  • 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
  • ✨ All others will result in a minor version release.

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Add option to IDEA plugin (and CLI) to format JavaDoc #724

Check the box to generate changelog(s)

  • Generate changelog entry

@blutorange
Copy link
Author

@schlosna As far as I can see, the changelog needs to be generated by somebody with the proper permissions for this repo?

@blutorange
Copy link
Author

Is there anything I still need to do before this PR can be considered for review?

@walles
Copy link

walles commented Apr 27, 2024

This branch has conflicts that must be resolved.

@blutorange
Copy link
Author

Thanks for notifying me! I resolved the conflicts in case anybody wants to take a look at getting this merged.

@blutorange
Copy link
Author

Is there any chance of getting this merged? Are you open to adding this option to the plugin or should I close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants