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

tracking issue: file-lines #1514

Closed
nrc opened this issue May 7, 2017 · 8 comments
Closed

tracking issue: file-lines #1514

nrc opened this issue May 7, 2017 · 8 comments
Labels

Comments

@nrc
Copy link
Member

nrc commented May 7, 2017

file-lines is an option to format only a range of a file, rather than a whole project. It is important for using rustfmt from editors or the RLS.

I must admit I'm not exactly sure on the status of the feature. I'll attempt to track what needs doing here.

cc @kamalmarhubi @topecongiro @fintelia

@nrc nrc added the p-high label May 7, 2017
@fintelia
Copy link
Contributor

It is probably worth clearly defining what sorts of things should and shouldn't be formatted when they partially overlap a range specified by file_lines. There are tons of possible edge cases and it is worth making sure that the code makes consistent and reasonable choices throughout.

Another test that would be worth having would be one that run rustfmt over a large collection of files (perhaps all of the integration tests?) with file_lines set to the empty set. Ideally this should leave everything unchanged, but I suspect that there are places that forget to check file_lines at all before applying formatting.

@nrc
Copy link
Member Author

nrc commented May 14, 2017

It is probably worth clearly defining what sorts of things should and shouldn't be formatted when they partially overlap a range specified by file_lines

My perspective here is purely from IDEs, I'm not sure if there are other applications which have other needs.

I would expect that formatting with file lines would be expected to be approximate, so there would probably never be a spec for what gets formatted and what doesn't. I would expect the lines specified would be the minimum and that the maximum would be the entire file.

If there is a mod foo; declaration inside the specified lines, I would not expect foo to be reformatted unless it is explicitly covered by the specified lines.

I would expect any module-level AST-node which overlaps the specified lines to be formatted. That is, any AST node which can appear in a module (and thus can be parsed alone). This covers functions, consts/statics, modules, data structures, impls, and traits. I would expect that only the smallest module-level node would be reformatted. E.g.,

1|mod a {
2|  mod b {
3|    struct c;
4|    struct d;
5|  }
6|}

lines: 4 would only reformat d, lines: 3-4 would reformat c and d, lines: 2-3 would reformat b, including c and c, and lines: 1 would reformat everything.

In the future we might change this to any AST node, rather than any module-level node, but that will require fairly deep changes to both the parser and Rustfmt.

I would expect comment formatting to be a bit rough around the boundary. Basically, if a comment is inside the lines but outside any AST reformatted nodes, or outside the lines, but inside (or adjacent to) a reformatted node, then the comments may or may not be reformatted.

I feel like there should be more. What am I missing?

@kamalmarhubi
Copy link
Contributor

kamalmarhubi commented May 21, 2017

I must admit I'm not exactly sure on the status of the feature.

Unless someone has done any work with it in the last year since I touched it, the status is that it's very much incomplete. I believe that I implemented argument parsing and threading the file lines through the context, as well as a not-really-reliable proof of concept for statements.

I would expect any module-level AST-node which overlaps the specified lines to be formatted.

Does this mean Items? I think this is too much. If I modify the signature of a function, I would not want its body to be modified. Similarly, renaming a module should not reformat its contents. I think a more human-centered case-by-case examination of AST structures is a better idea.

@nrc
Copy link
Member Author

nrc commented May 21, 2017

Unless someone has done any work with it in the last year since I touched it, the status is that it's very much incomplete

There has been some work - @fintelia has done some good stuff recently

If I modify the signature of a function, I would not want its body to be modified. Similarly, renaming a module should not reformat its contents. I think a more human-centered case-by-case examination of AST structures is a better idea.

This seems doable as a long term thing, but I think it pretty tricky to get into a 'good-enough' milestone.

@da-x
Copy link
Member

da-x commented Jun 28, 2018

Dear members,

What is the most stable revision of rustfmt with support for --file-lines at the moment?

I have tried the nightly-mode master branch (Built via CFG_RELEASE_CHANNEL=nightly cargo +nightly install --force) and detected some breakage.

Background: I am currently maintaining rust-lang/rust.vim and trying to integrate with a working version for the RustFmtRange command.

Given the input a.rs:

fn main() {
       let a = 2;

    let x = 1;

    let c = 2;

       let b = 2;

    let d = 3;
}

Both of the following two modes of executions that I am trying do not work:

1

rustfmt --emit files --unstable-features --file-lines '[{"file":"a.rs","range":[5,10]}]' a.rs

Result: The entirety of a.rs is reformatted, even though a range is provided.

2 (older method)

rustfmt --emit files --unstable-features --file-lines '[{"file":"a.rs","range":[5,10]}]'

Result: rustfmt waits for stdin and does nothing to a.rs. If I prepend echo | nothing happens to a.rs, and the following is printed: Warning: Extra file listed in file_lines option '/tmp/dan/a.rs' (my current directory is /tmp/dan.

Thanks!

@fintelia
Copy link
Contributor

The current implementation overestimates the lines to format. I don't recall the exact logic used, but in your example, this causes everything in the main function to be reformatted. If you added additional functions to that same file, those should not get formatted.

@da-x
Copy link
Member

da-x commented Jun 28, 2018

Thanks, that really clarifies :)

@ismell
Copy link

ismell commented Jan 10, 2022

What is the expected format of the --file-lines arg when the file to format is passed in via stdin? I tried using - as the filename, but that results in Can't canonicalize -.

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

No branches or pull requests

6 participants