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

RFC: Support formatting only changed regions #34

Open
daniel-grumberg opened this issue Feb 28, 2021 · 10 comments
Open

RFC: Support formatting only changed regions #34

daniel-grumberg opened this issue Feb 28, 2021 · 10 comments

Comments

@daniel-grumberg
Copy link

What I have in mind is using something like what highlight-changes-mode does on save to track the modified lines. We could then use the current RCS patch strategy to match diff hunks to modified regions (essentially if the diff is contained within the modified region plus some configurable context value) and only apply these hunks.

I am happy to develop this but I want to first here this would be of interest to people. Also I am relatively new to elisp so it might take some time and I might need some guidance navigating the existing source.

@raxod502
Copy link
Member

raxod502 commented Mar 7, 2021

This seems like a fine addition. I'd suggest implementing it as two different components which can then be combined together to form a complete solution:

  • the ability to limit formatting to a given region (perhaps the user wants to format the active region only)
  • some way of determining what regions have changed since the last save

@daniel-grumberg
Copy link
Author

Having thought about it more (I implemented something similar to this in my own config deferring to lsp for the actual formatting), formatting just a region has to be best effort. The thing you hope for is that you just need to apply the hunks that overlap with the target region. In general this is not sound because you might need some other hunks depending on language semantics. Let me know if you are still happy with implementing the functionality in a best effort manner.

@raxod502
Copy link
Member

raxod502 commented Mar 8, 2021

I think that's perfectly reasonable---there is a failure condition, but it's the kind of thing that makes a lot of sense in hindsight (what else did you expect the software to do?), and I am okay with software that fails sometimes as long as it fails for reasons that make sense and are easy to predict.

@mohkale
Copy link
Contributor

mohkale commented May 9, 2022

I've wanted this as well, but my desire for it is starting to wane. I'd recommend reading black#134. It provides a pretty compelling reason IMO for why this isn't supported. I'd say consistency is more valuable than any of the reasons for avoiding formatting the entire file. If you're adopting a formatter you should use it on everything.

@lassik
Copy link

lassik commented May 10, 2022

In format-all, I opted to only support format-region using formatters which have official command line flags to support that feature. Otherwise we open a can of worms, as you partially sketched above.

A few formatters already do have the feature. In all of those cases, the entire file is sent to the formatter as usual. The formatter outputs the entire file with changes only in the given region.

For reference:

@raxod502
Copy link
Member

I agree with that assessment. Thanks for the suggestion!

@VitalyAnkh
Copy link

Any updates on this? Wishing for this feature to be implemented 😊

@raxod502
Copy link
Member

raxod502 commented Mar 3, 2023

My current support focus is Radian, and after that I will likely be prioritizing straight.el. After that, the Apheleia backlog may be a next step, but it depends on how things develop. As always, community patches will be reviewed and merged.

@xeechou
Copy link

xeechou commented Oct 30, 2023

I implemented this feature based off format-all-region, it's a small minor mode called fmo-mode (format-modified-only). The idea there is similar to @daniel-grumberg described. Extracting diff hunks and applying format-all-region in those regions. It's hooked off before-saving-hook. Hope it will help.

@hrehfeld
Copy link

(This feature would be great)

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

No branches or pull requests

7 participants