-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat(formater): restrict formatting to only the changed range of a file. #4604
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
base: dev
Are you sure you want to change the base?
Conversation
917250b to
f4593c6
Compare
f1dc981 to
3e15a39
Compare
df8bdf9 to
0dd5039
Compare
|
@rekram1-node What is the state on this? Are you considering these suggestions? |
|
Yeah I see what you are trying to solve there. I would need some time to look into it more to review but I will say this looks very vibe coded so I won't just auto merge without a throughout review |
Yes, I did use But yeah, this code needs to be reviewed, of course :) I started with the line-number-based range API of clang-format and actually left it as a separate commit (the rest is squashed into the subsequent commit). Then I checked which interface Qt Creator uses: char/byte range API for subfile formatting --- and as it enables us to apply the same logic with Aside from my use case (legacy code bases with humongous unformatted areas): Auto-formatting only the affected part of the file that opencode had touched makes a lot of sense to me; formatting the rest of the file should be a different task / step anyways IMO. Having such a system in place would not only be very beneficial for my workflow, I believe it would make opencode a better tool for everyone. Feel feel to request changes or I'd also be happy to hand this branch over. |
|
@rekram1-node Is there any way I can increase your confidence with that topic? |
f8ee907 to
6a9856d
Compare
d05f216 to
5b0fb78
Compare
@rekram1-node bump :) |
|
this is good we're gonna merge it, aiden will follow up |
|
@micuintus can you update your code to follow some of our style guidelines a bit better? Ex: change changedRanges to just ranges, stuff like that |
5bd0b08 to
762626d
Compare
@rekram1-node Sorry for the delay, I only just saw this. -> Done: 2d47783 Anything else? |
|
/review |
|
lgtm |
|
u can ignore that fail its whatever, ill merge tonight |
When using the Edit tool, clang-format now only formats the specific lines that were changed, rather than reformatting the entire file. This prevents unrelated formatting changes from cluttering the diff. Adds calculateChangedLines() to detect modified line ranges from diffs, and updates clang-format to use --lines flag for targeted formatting. Other formatters continue to format the entire file as before. Closes sst#4603
Switch from line-based to exact character/byte range formatting with a robust DiffRange class. This refactoring improves compatibility across different formatters and adds comprehensive validation. Changes: - Replace line ranges with exact character and byte offset tracking - Implement DiffRange class to encapsulate range logic and conversions - Add support for Prettier (character offsets) and clang-format (byte offsets) - Handle Unicode multi-byte characters correctly - Merge adjacent ranges to reduce formatter invocations - Add validation to fromJSON() for negative/invalid ranges - Extract clampOffset() helper to reduce code duplication - Remove unused toByteOffsets() method - Cache Buffer.byteLength() calls to avoid redundant computation - Fix duplicate getCachedByteOffsets() calls in merge() Ref sst#4603
…ature - calculateChangedRanges -> calculateRanges - Simplify variable names (charOffset etc.) - Replace else statements with early returns - mergeAdjacentRanges -> mergeRanges Style guide: https://github.com/sst/opencode/blob/dev/STYLE_GUIDE.md Ref sst#4603
|
/review |
|
lol only i can trigger it |
LOL #FAIL :)
Anything else I can do to help? |
|
Hm? |
Restrict formatting only to the edited line range for clang-format
When using the Edit tool, clang-format now only formats the specific lines
that were changed, rather than reformatting the entire file. This prevents
unrelated formatting changes from cluttering the diff.
Closes #4603