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

Partial code formatting #191

Open
vchuravy opened this issue Feb 25, 2020 · 13 comments
Open

Partial code formatting #191

vchuravy opened this issue Feb 25, 2020 · 13 comments
Labels
enhancement New feature or request

Comments

@vchuravy
Copy link

vchuravy commented Feb 25, 2020

It is very convenient to apply formatting changes to only partial files. As an example clang-format can integrated with git into https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/clang-format-diff.py the actual git integration is here https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/git-clang-format
cc: @simonbyrne

@StefanKarpinski
Copy link
Contributor

It's a bit unclear to me how this should work (and from reading that Python script how it does work). A straightforward way to do this would be to generate a zero-context diff (that appears to be what they do here) and apply that to the formatted version of the original and then if that applies cleanly also format the modified file and then generate a zero-context diff between the two formatted versions and apply that diff to the original file. In commutative diagram form:

A ---> B
|      |
v      v
A'---> B'

Here A is the original version of the file, B is the new version and A' and B' are the formatted versions of A and B. We want to apply the diff from A' to B' to A to get a new version of B that is partially formatted but only the code that the diff touches. But what if the diff from A' to B' doesn't apply to A cleanly? How would one handle that?

@tkf
Copy link

tkf commented Feb 26, 2020

IIUC there is no magic in clang-format-diff.py. It just uses VCS' diff command to construct --lines options passed to clang-format

  --lines=<string>           - <start line>:<end line> - format a range of
                               lines (both 1-based).
                               Multiple ranges can be formatted by specifying
                               several -lines arguments.
                               Can't be used with -offset and -length.
                               Can only be used with one input file.

--- https://clang.llvm.org/docs/ClangFormat.html

So, I guess JuliaFormatter.format and JuliaFormatter.format_text need to have a similar option to handle this?

@domluna
Copy link
Owner

domluna commented Feb 26, 2020

I suppose this could be done in the printing phase by toggling formatting. If formatting is off the original content for those lines is printed.

@StefanKarpinski
Copy link
Contributor

Doesn't that require that the formatter not split or combine lines?

@simonbyrne
Copy link

A potentially simpler option would be to output the results of the check by file, and compare that against the list of files modified in the PR.

@domluna domluna added the enhancement New feature or request label Feb 29, 2020
@domluna domluna pinned this issue Oct 21, 2020
@domluna domluna unpinned this issue Nov 29, 2021
@domluna domluna pinned this issue Feb 12, 2022
@tkf
Copy link

tkf commented Mar 22, 2022

FYI, I just found https://github.com/reviewdog/action-suggester which can be used to suggest formatting for the parts of the code that the PR touches. See JuliaPreludes/Try.jl#21 for an example setup. See JuliaPreludes/Try.jl#22 for how it looks like. I've seen some edge cases for detecting what the PR touches, though (example https://github.com/tkf/DisplayAs.jl/pull/23/files#r826040308)

@domluna domluna unpinned this issue Apr 19, 2022
@fingolfin
Copy link

Doesn't that require that the formatter not split or combine lines?

AFAIK the way git clang-format handles this is by taking such changes if any of the affected lines was modified. I.e. if my patch modifies a line and JuliaFormatter combines that line with another, or splits it, then this is taken. So I guess one would need to store for each output line which input lines it originates from (typically this will be a single line, but sometimes multiple).

A potentially simpler option would be to output the results of the check by file, and compare that against the list of files modified in the PR.

But that only helps if those modified files are otherwise fully formatted, right? That doesn't sound that useful to me...

@fingolfin
Copy link

I wonder if the code used for the #! format: off support could be adapted to handle this. Basically, the information extracted from the git diff could be used to insert "virtual" skip formatting markers: skip formatting at the start of the file, until the first modified lines, etc. Wonder how hard it would be to cook up a rough prototype based on that...

Things get more complicated once there are real "skip formatting" comments, though... I don't know if current machinery dealing with format_skips ranges is prepared for them to overlap?

@sloede
Copy link

sloede commented Nov 21, 2022

Has there been any update on this issue yet (it doesn't look like it)? The ability to apply partial formatting would imho immediately increase the usefulness of JuliaFormatter for an automatic CI check. Why? For a large code base, it is usually unfeasible to apply formatting to the entire source: It destroys convenient git blame functionality, and it will be a recurring headache if the authors of the code formatter tool will change anything that affects my current format.

However, it would awesome to be able to apply formatting to git diffs only. This allows one to ensure in a CI pipeline that a proposed PR will not change after formatting the diff, i.e., forcing people to propose only PR code that has already been formatted. Especially in packages with a non-negligible number of drive-by PRs, this could considerably reduce the burden on the maintainers.

@efaulhaber
Copy link
Contributor

Any updates on this? It seems to me that this features already exists, since VS Code has both “Format Selection” and “Format Modified Lines” features, but I haven't managed to find out how they do it.
Could anyone point me in the right direction?

@domluna
Copy link
Owner

domluna commented Jan 16, 2023

Any updates on this? It seems to me that this features already exists, since VS Code has both “Format Selection” and “Format Modified Lines” features, but I haven't managed to find out how they do it.

Not natively supported yet, not sure how vscode does it. Probably something similar to where they ignore the formatting for all the lines but the selected ones.

@davidanthoff @pfitzseb ☝️

@efaulhaber
Copy link
Contributor

Unfortunately, it seems like that is a native VS Code feature.

@flwyd
Copy link

flwyd commented Nov 25, 2023

I added line-range formatting for Julia in the vim-codefmt Vim plugin in google/vim-codefmt#230 by inserting and then removing #! format: off and #! format: on around the affected line ranges. The Julia script which accomplishes this could be adapted for general use.

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

No branches or pull requests

9 participants