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

[unstable option] file_lines #3397

Open
scampi opened this issue Feb 13, 2019 · 12 comments
Open

[unstable option] file_lines #3397

scampi opened this issue Feb 13, 2019 · 12 comments
Labels
stabilisation-request A request to stabilise an option or argument unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for file_lines

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@scampi
Copy link
Contributor Author

scampi commented Feb 21, 2019

original issue: #1514

@Xanewok
Copy link
Member

Xanewok commented Mar 1, 2019

Will this option be stabilized anytime soon? We use it for quite some time in RLS for the Range Formatting and it seems to work without any problems.

One thing that sometimes happens IIRC is that it can "grab" remaining parts of the selected expression/AST node outside of the specified range by 1 or 2 lines but it always seemed like it did a reasonable thing, e.g. it was hard to imagine how you could only partially format the specified range.

RLS also uses feature gating for this, so if it were made stable we could offer it to our users out of box, without them having to opt-in into unstable features. cc @nrc

@scampi scampi added the stabilisation-request A request to stabilise an option or argument label Mar 4, 2019
@scampi
Copy link
Contributor Author

scampi commented Mar 12, 2019

One thing that sometimes happens IIRC is that it can "grab" remaining parts of the selected expression/AST node outside of the specified range by 1 or 2 lines but it always seemed like it did a reasonable thing, e.g. it was hard to imagine how you could only partially format the specified range.

@Xanewok Sounds like that is by design, see #1514 (comment) and #1514 (comment).

I have combed through the issues mentioning or related to file-lines in order to get a better idea of the current state of that option. Here is an overview and my take on them:

Overall, I would say 2 issues are needed to complete stabilisation. @topecongiro ?

@jsgf
Copy link

jsgf commented Mar 13, 2019

#3442 is a blocker for us to be able to use --file-lines at all - it produces inconsistent and bimodal results.

There's also another bug I haven't narrowed down yet where rustfmt seems to (sometimes) infinitely indent the internals of macro definitions, which could also be --file-lines related.

@nrc
Copy link
Member

nrc commented Mar 27, 2019

Looks like #3442 is fixed now. It is by design that file lines 'rounds up'. We need to round up to the nearest AST node in the best case and we are currently very conservative (I think round up to the nearest item, or maybe even module). So while we could do better, I don't think we need to block stabilisation (although we would need to think about what exactly stability means then). Note that the LSP spec permits rounding up.

@Xanewok
Copy link
Member

Xanewok commented Apr 5, 2019

@scampi is there any issue left that's specifically related to file_lines which blocks this stabilization?

@scampi
Copy link
Contributor Author

scampi commented Apr 8, 2019

@Xanewok I believe #1873 is the outstanding issue

bors bot added a commit to intellij-rust/intellij-rust that referenced this issue Sep 16, 2020
5704: FMT: Add an option to use rustfmt instead of build-in formatter r=vlad20012 a=mchernyavsky

Closes #5191. c.c. #1087

In this implementation, `Rustfmt` can be used **only** when formatting the **whole file** by: 
1. `Code | Reformat Code` (`Ctrl + Alt + L`) action **without text selection**
2. `Code | Reformat File ...` (`Ctrl + Alt + Shift + L`) action with `Whole file` option

E.g. `Rustfmt` can't be used when committing to git with `Reformat code Before Commit` option (it doesn't clear how to achieve this).

By default, the built-in formatter is still ALWAYS used. `Rustfmt` can be enabled in `Settings | Languages & Frameworks | Rust | Rustfmt`.

NB: rustfmt has an unstable option to restrict reformatting to specific sets of lines (rust-lang/rustfmt#3397).

Co-authored-by: vlad20012 <[email protected]>
Co-authored-by: mchernyavsky <[email protected]>
bors bot added a commit to intellij-rust/intellij-rust that referenced this issue Sep 16, 2020
5704: FMT: Add an option to use rustfmt instead of build-in formatter r=vlad20012 a=mchernyavsky

Closes #5191. c.c. #1087

In this implementation, `Rustfmt` can be used **only** when formatting the **whole file** by: 
1. `Code | Reformat Code` (`Ctrl + Alt + L`) action **without text selection**
2. `Code | Reformat File ...` (`Ctrl + Alt + Shift + L`) action with `Whole file` option

E.g. `Rustfmt` can't be used when committing to git with `Reformat code Before Commit` option (it doesn't clear how to achieve this).

By default, the built-in formatter is still ALWAYS used. `Rustfmt` can be enabled in `Settings | Languages & Frameworks | Rust | Rustfmt`.

NB: rustfmt has an unstable option to restrict reformatting to specific sets of lines (rust-lang/rustfmt#3397).

Co-authored-by: vlad20012 <[email protected]>
Co-authored-by: mchernyavsky <[email protected]>
@joshtriplett
Copy link
Member

All of the issues mentioned as blockers seem to have been addressed. Is there any remaining blocker to stabilizing --file-lines?

@calebcartwright
Copy link
Member

All of the issues mentioned as blockers seem to have been addressed. Is there any remaining blocker to stabilizing --file-lines?

I believe #4053 may contain some issues that need investigation, but haven't looked to closely.

@euclio
Copy link
Contributor

euclio commented May 4, 2021

Can this be used with stdin?

EDIT: I see that you can provide "file": "stdin", but I don't see this documented.

@euclio
Copy link
Contributor

euclio commented Aug 1, 2021

I've been using this via rls and rust-analyzer, and haven't noticed any issues. I'm interested in seeing this stabilized.

@not-my-profile
Copy link

As I reported in #5136 file-lines currently changes whitespace outside of the given range. That should be fixed before stabilization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stabilisation-request A request to stabilise an option or argument unstable option tracking issue of an unstable option
Projects
None yet
Development

No branches or pull requests

8 participants