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

For unlinked files, add option to add "pub mod" instead of just "mod" #8228

Closed
marcusbuffett opened this issue Mar 29, 2021 · 1 comment · Fixed by #8345
Closed

For unlinked files, add option to add "pub mod" instead of just "mod" #8228

marcusbuffett opened this issue Mar 29, 2021 · 1 comment · Fixed by #8345
Labels
A-assists A-diagnostics diagnostics / error reporting E-easy S-actionable Someone could pick this issue up and work on it right now

Comments

@marcusbuffett
Copy link

It seems to be a pretty common pattern to have mod.rs files that are just a bunch of pub mod x, pub mod y etc., so it would be nice when you create an unlinked file and use the code action from rust-analyzer, if in addition to Insert 'mod x;', it would have a Insert 'pub mod x;' option. Super useful feature already, just think this would make it even better :)

Also you guys probably hear this a million times a day but rust-analyzer is amazing; it's totally changed the rust experience for me, and the pace of development is unreal.

@Veykril Veykril added A-assists A-diagnostics diagnostics / error reporting E-easy S-actionable Someone could pick this issue up and work on it right now labels Mar 29, 2021
@yue4u
Copy link
Contributor

yue4u commented Apr 3, 2021

Hi, I would like to work on this and just took a look at some relevant code. The Diagnostic hint seems to be built here.

https://github.com/rust-analyzer/rust-analyzer/blob/2f87ee7f3f9c1efd726774511c9ce02caef73872/crates/ide/src/diagnostics.rs#L161-L171

However, It looks like DiagnosticWithFix is limited to a single fix.

https://github.com/rust-analyzer/rust-analyzer/blob/2f87ee7f3f9c1efd726774511c9ce02caef73872/crates/ide/src/diagnostics.rs#L59-L60

https://github.com/rust-analyzer/rust-analyzer/blob/2f87ee7f3f9c1efd726774511c9ce02caef73872/crates/ide/src/diagnostics/fixes.rs#L25-L30

In this situation I think appending a different Diagnostic is easier to do but does not make much sense, since it's the same problem and the fixes are so similar. It will be better to add / change to DiagnosticWithFixes and accept multiple possible fixes?

In addition, I found the the comment // Override severity and mark as unused. inside of .on::<UnlinkedFile, _>( ... does not match the code below and the item to append is same as the return value of warning_with_fix https://github.com/rust-analyzer/rust-analyzer/blob/2f87ee7f3f9c1efd726774511c9ce02caef73872/crates/ide/src/diagnostics.rs#L230-L234

Anyway I'm new to the rust-analyzer codebase and may read the code wrong, I'd be happy to continue working on this if some instructions are given.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists A-diagnostics diagnostics / error reporting E-easy 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.

3 participants