Skip to content

generate_derive loses indent #14674

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

Closed
lnicola opened this issue Apr 28, 2023 · 3 comments · Fixed by #14752
Closed

generate_derive loses indent #14674

lnicola opened this issue Apr 28, 2023 · 3 comments · Fixed by #14752
Assignees
Labels
A-assists C-bug Category: bug S-actionable Someone could pick this issue up and work on it right now

Comments

@lnicola
Copy link
Member

lnicola commented Apr 28, 2023

image

https://github.com/rust-lang/rust-analyzer/blob/816f7fe/crates/ide-assists/src/handlers/generate_derive.rs#L45

@lnicola lnicola added S-actionable Someone could pick this issue up and work on it right now A-assists C-bug Category: bug labels Apr 28, 2023
@ponyii
Copy link
Contributor

ponyii commented May 6, 2023

just for the sake of clarity:

generate_derive being called on a snippet like this eats an indent.

mod module {
    enum Enum {      // this indent will be lost
        Whatever,<|>
    }
}

@ponyii
Copy link
Contributor

ponyii commented May 6, 2023

I've found some other assists that doesn't taking into account indentation and, consequently, produce incorrectly indented code (e.g. toggle_ignore, generate_deref). It's worth checking them all.

Another problem is that each assist deal with indentation by itself (an example). It might be more convenient to write a single function for the generated code indentation.

I think I can fix it within this issue, could you assign it to me?

@lnicola lnicola assigned lnicola and ponyii and unassigned lnicola May 6, 2023
@ponyii
Copy link
Contributor

ponyii commented May 11, 2023

I've written a few ad hoc patches and some new tests (PR #14752), but there're still too many assists that can break indentation, so some general solution is required.

As far as I understand, all the assists eventually pass a closure to Assists::add that changes the source code using different methods of SourceChangeBuilder. None of these methods checks if the code addition / replacement / whatever breaks the indentation, and I suppose it's the root cause of the bug.
It doesn't seem to me convenient to introduce such a check in these methods (one reason: their arguments are impl Into<String>, but not easy-to-reindent syntax trees). The general solution I'd suggest is to call some fix_indentation function after each usage of an assist; though it's in no way brilliant - for instance, it will break any user's unconventional indentation.

(I believe this issue is no longer actionable and requires a discussion of the general solution - and whether it's worth implementing one at all)

@bors bors closed this as completed in a512774 May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists C-bug Category: bug S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants