Skip to content

[Enhancement] Only Send Updates When Formatting Changed #1188

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

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

Conversation

bmarcaur
Copy link
Member

@bmarcaur bmarcaur commented Jan 9, 2025

Before this PR

Every format action caused the current buffer to be marked as modified.

After this PR

This was due to us updating the text on every format even if it was unchanged. Now it only updates when something was formatted.

Way more context than you could ever want: https://youtrack.jetbrains.com/issue/IJPL-162382

@bmarcaur bmarcaur requested review from CRogers and crogoz January 9, 2025 21:04
@bmarcaur bmarcaur changed the title follow the API guidelines and only update when formatted file is changed [WIP] Only Send Updates When Formatting Changed Jan 9, 2025
@bmarcaur bmarcaur force-pushed the bmarcaurele/only-send-format-when-changed branch from ae2a2c2 to 8f96dc6 Compare February 27, 2025 21:57
@bmarcaur bmarcaur changed the title [WIP] Only Send Updates When Formatting Changed [Enhancement] Only Send Updates When Formatting Changed Feb 27, 2025
@bmarcaur bmarcaur marked this pull request as ready for review February 27, 2025 21:58
// The Javadoc of onTextReady API says that you should set it to null when the
// document is unchanged. But an even better version is to simply not attempt
// to format a document that is already formatted
if (replacements.isEmpty()) {
Copy link
Member Author

@bmarcaur bmarcaur Mar 6, 2025

Choose a reason for hiding this comment

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

Damn so it would turn out that even when the file is fully formatted it still produces a replacement which looks like:

image

A full file replacement with the same content... Seems like a bug in the formatter service.

CC @crogoz @carterkozak :(

Copy link
Member Author

Choose a reason for hiding this comment

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

To avoid having to remote debug your intellij, here is a failing test that exemplifies the issue: https://github.com/palantir/palantir-java-format/pull/1188/files#diff-a6622bd2788dface780176f19bbaecce6308bb0a826e6e6721b0c713bc0e2c68R450

@bmarcaur bmarcaur force-pushed the bmarcaurele/only-send-format-when-changed branch from 27dfe25 to 70d44a5 Compare March 6, 2025 18:45
@bmarcaur bmarcaur force-pushed the bmarcaurele/only-send-format-when-changed branch from 70d44a5 to b2fb695 Compare March 6, 2025 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants