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

Do not modify the buffer if yapf provides no change. #30

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

Conversation

ChengYuShun
Copy link

Currently the buffer being formatted will always be modified when yapfify-region is run. This patch simply checks whether any changes are made before modifying the buffer, avoiding unnecessary edits.

@JorisE
Copy link
Owner

JorisE commented Sep 11, 2023

Hi @ChengYuShun Thanks for your interest in this package and your suggestion! I'm not sure I think it would be wise to add this check. I agree that this may lead to unnecessary edits, but only to the buffer. What do you think would be saved by not editing the buffer?
I had to dive very deep into my memory, and I remember that yapifify broke at some point because the error codes for YAPF changed. That is when this solution was introduced. I think there have been several YAPF versions since than, maybe a solution is now also available in YAPF itself?

@ChengYuShun
Copy link
Author

ChengYuShun commented Sep 14, 2023

I agree that the current approach perfectly fine, since given that before-save-hook is run, the buffer must have already been modified, and overwriting won't cause anything undesirable.
However, I personally would prefer to bind yapfify-buffer with a shortcut, and run it manually. If yapfify chooses to overwrite the buffer even when no changes are made, I will always have to save it again afterwards. I don't know whether this is the expected way of using the package, but most IDEs use key bindings to format code, so I guess this patch will make some people's life easier. Besides, an additional check won't make yapf-mode less efficient either.

@JorisE
Copy link
Owner

JorisE commented Sep 18, 2023

Hi, I think I don't understand your usecase completely, you say: "If yapfify chooses to overwrite the buffer even when no changes are made, I will always have to save it again afterwards. ".
Why do you have to safe the buffer if no changes were made to it?

@ChengYuShun
Copy link
Author

The thing is that yapfify always overwrites the buffer, so the buffer is modified even if no actual changes were made.
For instance, if you delete a character and then add it back, the buffer would be modified, and you will have to either save the buffer or undo the changes when you quit Emacs. In order to reproduce this, you could open an already well-formatted python file, call yapfify-buffer interactively, and observe two of the dashes in the bottom left corner turn into asterisks, indicating that this buffer has been modified. If you quit Emacs now, you would get asked whether or not to save this buffer.
Technically, you do not have to save it again. You can simply undo the 'changes'. But I just feel it would be nice if the buffer just remains unmodified.

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.

2 participants