Rust: attributes on grouped imports are not preserved
interruptinuse opened this issue · 10 comments
(Follow-up to #189, but a different issue) Preceding attributes on grouped imports are not preserved when those imports are split or joined:
(█
marks position of the cursor)
Splitting:
#[cfg(test)]
use █crate::{import1, import2};
// Splits into
#[cfg(test)]
use █crate::import1;
use crate::import2;
// But should split into
#[cfg(test)]
use █crate::import1;
#[cfg(test)]
use crate::import2;
Joining with attributes doesn't work either, but that one is a harder issue, since it's invalid to join imports with different attributes (and AFAIK, placing attributes inside curly braces doesn't work):
#[cfg(test)]
use █crate::import1;
#[cfg(test)]
use crate::import2;
// Joins into
#[cfg(test)]
use crate::import1;█#[cfg(test)]
use crate::import2;
// Ideally, would join into this
#[cfg(test)]
use █crate::{import1, import2};
I've created a branch called rust-import-attributes
where I've implemented handling of attributes above imports. I think it should handle both splitting and joining correctly. As you say, if the attributes are not the same, they're not joined.
Theoretically, something like #[derive(Clone, Default)]
would be equivalent to #[derive(Default, Clone)]
, but I think it's good enough to insist that they'll only join if they're exactly identical. There's limits to what's practical to do inside the plugin.
Could you try it out and let me know if I've missed some edge case or if I've broken something else?
Hi, thanks for the quick fix!
I think it's good enough to insist that they'll only join if they're exactly identical.
Yeah, I think so as well, semantics-aware formatting should be left to language-specific tools. So in terms of practicality it's completely understandable.
Issues I found:
- Joining imports loses a trailing newline; not related to cd52bfe (#189 (comment)), happens without that commit, more likely e14a613. Also seems to lose the cursor position.
#[cfg(test)]
use █std::collections::HashSet;
#[cfg(test)]
use std::collections::HashMap;
use std::error::Error;
// Is joined into
#[cfg(test)]
use std::collections::{HashSet, HashMap};█
use std::error::Error;
- Indented imports (such as in functions) are not handled correctly:
pub fn function() {
#[cfg(target_feature = "bmi1")]
use █std::collections::{HashSet, HashMap};
}
// Is split into
pub fn function() {
#[cfg(target_feature = "bmi1")]
use █std::collections::HashSet;
use std::collections::HashMap;
}
pub fn function() {
#[cfg(test)]
use █std::collections::HashSet;
#[cfg(test)]
use std::collections::HashMap;
}
// Is joined into
pub fn function() {
#[cfg(test)]
use std::collections::HashSet;█ #[cfg(test)]
use std::collections::HashMap;
}
Regarding 1, I think it was the same problem, I just hadn't merged the fix in main here -- should be fine now. For 2, I'd forgotten that case, adjusted the regex. Let me know if the current state of the branch works for you and I'll merge it in.
Thanks for the fix!
As of a6a366e, 1.1 (from the comment above) and 2.1 now work as expected. Indented join (2.2) merges attributes correctly, but seems to be eating the trailing newline:
fn foo() {
#[cfg(test)]
use █std::collections::HashMap; // Join here
#[cfg(test)]
use std::collections::HashSet;
}
// Results in
fn foo() {
#[cfg(test)]
use std::collections::{HashMap, HashSet};}
Hmm, this is a problem -- I can't replicate the issue. Joining the first with the cursor where you indicated gives me the second:
fn foo() {
#[cfg(test)]
use std::collections::HashMap;
#[cfg(test)]
use std::collections::HashSet;
}
fn foo() {
#[cfg(test)]
use std::collections::{HashMap, HashSet};
}
This is probably a difference in configuration. Could you share your vimrc? I'd like to try using your config to replicate it, see what it could be that's causing the difference. It would also help if you could post your Vim version (first two lines of vim --version
)
Interesting.
gJ
does not work as intended; neither does:call <SNR>98_Mapping(g:splitjoin_join_mapping, "<SID>Join")
; (98
is~/.config/nvim/plug/splitjoin.vim/plugin/splitjoin.vim
):SplitjoinJoin
does work as intended;- So does
:nnoremap gJ :SplitjoinJoin<CR>
thengJ
;
My configuration is, uh... a bit of an outdated mess.
$ nvim --version
NVIM v0.7.0
Build type: Release
LuaJIT 2.1.0-beta3
Compiled by nixbld
Features: +acl +iconv +tui
See ":help feature-compile"
system vimrc file: "$VIM/sysinit.vim"
fall-back for $VIM: "
/nix/store/8s7xlazpgfip0bfakcjvsa20zz11m0ga-neovim-unwrapped-0.7.0/share/nvim
"
Run :checkhealth for more info
Update: can reproduce in this neovim with clean config:
set runtimepath^=~/.config/nvim/plug/splitjoin.vim
Seeing as how it's probably a different issue and the actual command behaves correctly, I think I'm going to go ahead and close this one? Thanks again for the fix!
Well, I'd still like to figure it out if I can. I'll try to put some time in it at some point.
@interruptinuse This was a subtle bug on my side. All the callbacks should return 1 when they succeed, and 0 if they fail. This one was accidentally returning 0. The reason the command works is the callback was working just fine. The mapping, however, checks the result and runs the default mapping if it gets a 0 as a fallback. Which happens to be one extra join.
I wasn't hitting the bug, because in my config, the default mapping is disabled anyway 😅. Tests weren't catching it, because they run the command, not the mapping. I've pushed a fix, and maybe I'll see about executing the mapping instead of the command in tests.
@AndrewRadev Oh cool thanks! WFM