[vim/vim] rust.vim: stop setting formatprg to rustfmt. (PR #18650)
Aaron Jacobs
This reverts commit 4ac995b.
This was added in #16807, with no explanation for why it was necessary beyond "it's an example of an idea". It completely breaks gq for me—rustfmt doesn't reflow comments so is not an appropriate tool here! Beyond that, formatting a selection with rustfmt treats that selection as if it were an entire file, throwing away any indentation.
For example, the commit causes gq to turn this:
pub fn foo() { // blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah }
into this:
pub fn foo() { // blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah }
which is totally wrong. In contrast, if I clear formatprg then gq does the right thing again:
pub fn foo() { // blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah // blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah // blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah // blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah // blah blah blah blah blah blah }
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/18650
Commit Summary
- 42b595c rust.vim: stop setting formatprg to rustfmt.
File Changes
(2 files)
- M runtime/autoload/rustfmt.vim (15)
- M runtime/ftplugin/rust.vim (16)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#18650)gq is meant to be used with proper formatprg, you can use gw instead to do what gq does without formatprg. (almost the same, it doesn't move the cursor).
However, formatprg should be able to work with a range, if it doesn't, it should be fixed/amended.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aaron Jacobs
jacobsa left a comment (vim/vim#18650)rustfmt does not work with ranges. It's not an appropriate choice for formatprg, whether or not you accept that gq is meant to use formatprg.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#18650)rustfmt does not work with ranges. It's not an appropriate choice for
formatprg, whether or not you accept thatgqis meant to useformatprg.
if it doesn't, it shouldn't be set as formatprg.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
Konfekt left a comment (vim/vim#18650)rustfmt does not work with ranges
Then that's my bad, I assumed it would read stdin
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Enno
Konfekt left a comment (vim/vim#18650)Which is a bit of a pity as it sounds that there's no way to format a code block other than reformatting the whole file?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aaron Jacobs
jacobsa left a comment (vim/vim#18650)Correct, that's my understanding. As a result this change completely breaks gq in (almost?) every case—for example, gqq to reflow the current comment line will both throw away its indentation and not reflow it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
Konfekt left a comment (vim/vim#18650)So yes, I just assumed rustfmt takes stdin, but not so, and this should be reverted
gqq to reflow the current comment line
For formatting comments likely the built-in C reflow gw is intended; that's what in practice most use gq for in code. Here gq was meant to help to format uncommented code, but that went awry
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
D. Ben Knoble
benknoble left a comment (vim/vim#18650)As I mentioned elsewhere:
FWIW, I set
formatprg=rustfmtmyself for Rust. Like with {Black and Python} or {Go}, you have to format enough of the code for the formatter to understand it (so typically a full top-level definition). I don't see anything wrong with this, personally.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18650)Alright, I'll revert it then.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Closed #18650.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18650)included as of eba5133
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aaron Jacobs
jacobsa left a comment (vim/vim#18650)@chrisbra: Thanks!
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yee Cheng Chin
ychin left a comment (vim/vim#18650)FWIW, I set
formatprg=rustfmtmyself for Rust. Like with {Black and Python} or {Go}, you have to format enough of the code for the formatter to understand it (so typically a full top-level definition). I don't see anything wrong with this, personally.
I don't really use Go, but just tested it out and seems like gofmt does respect leading indentation, which is what this issue was complaining about. If you select an indented line and then do gqgq in a Go file, it does work as expected. If you select multiple lines it seems to just treat the first line's leading indentation as the "default" indentation and go from there. It's not going to work in every single situation but that's why we have gw (gq is explicitly documented to use formatprg afterall). But then a single-line format should be expected to work.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
dkearns left a comment (vim/vim#18650)This was added in #16807, with no explanation for why it was necessary beyond "it's an example of an idea".
It was added because formatting text with the appropriate language formatter is better than using Vim's internal formatter for everything, especially when we can have both.
rustfmt does not work with ranges. It's not an appropriate choice for
formatprg, whether or not you accept thatgqis meant to useformatprg.
Executing gqq on the comment line in foo is passing that line alone to rustfmt for formatting as a single line range. Now, what is a formatter supposed to do with a single line comment? rustfmt assumes it's a top level comment and removes the indent while gofmt guesses the other way and leaves the indent alone. If you want the comment indented correctly in the context of the function definition you need to send the three line range for formatting via gq.
If you configure rustfmt to wrap comments the comment line would, of course, also be wrapped appropriately.
There is nothing special or broken about rustfmt. All formatters exhibit this sort of behaviour, they can only work with the input they're given.
I don't understand why "use gw" is being consistently ignored.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
dkearns left a comment (vim/vim#18650)Sorry my reply was sent before I noticed yours @ychin.
I don't really use Go, but just tested it out and seems like gofmt does respect leading indentation,
It appears to leave comment indent alone without any extra context though it doesn't wrap them, at least by default. Try it on the Println() line, alone, and it will remove the indentation too.
func main() { fmt.Println("Hello, World!") }
formatprg is, in these cases, an external program and the "expected behaviour" depends on the specific program. This is, I think, a desirable feature and we're tossing the baby with the bath water here.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aaron Jacobs
jacobsa left a comment (vim/vim#18650)I don't understand why "use gw" is being consistently ignored.
Because it's not relevant. The fact of the matter is that there was a major behavior change here, without any serious justification for why the behavior should change. You and others keep ignoring that, which is the relevant part. Telling me to change my muscle memory is not helpful.
I've been using vim for over 15 years, and in all that time highlighting a comment block and pressing gq has always reflowed the comment without screwing up the indentation. Now it screws up the indentation and doesn't reflow the comment. It has broken the functionality of gq, at least for the particular use case of formatting comments.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
dkearns left a comment (vim/vim#18650)Because it's not relevant. The fact of the matter is that there was a major behavior change here, without any serious justification for why the behavior should change. You and others keep ignoring that, which is the relevant part.
This user seemed much more open to its relevancy.
The change was obviously made because it was believed that having gq format Rust source code with the official Rust source code formatter might be more useful than the default behaviour. This does not seem as whimsical an idea as you keep suggesting. The trade-offs were considered at some length. The participants are not fools. There's also work being done on adding formatter plugins.
Filetype plugins are, by definition, supposed to override defaults with language specific behaviour. Not everyone finds every addition desirable.
I'm certainly not ignoring your issue, that would be difficult given how strident you've been in stating it from the beginning. I just don't think you're right.
A better solution to these issues might be to use formatexpr to mitigate the limitations of external formatters in replicating some of the more commonly used behaviour of the internal formatter.
Another reasonable improvement would be to move the setting of formatprg to behind the no_rust_maps guard.
Telling me to change my muscle memory is not helpful.
Well I am sorry as we are, of course, here to help.
I've been using vim for over 15 years, and in all that time highlighting a comment block and pressing gq has always reflowed the comment without screwing up the indentation. Now it screws up the indentation and doesn't reflow the comment. It has broken the functionality of gq, at least for the particular use case of formatting comments.
Yes, and improved it for other use cases. With fifteen whole years of experience there are surely a myriad of trivial solutions you could have implemented in less time than it took to post this issue, none of which require any brain plasticity whatsoever.
Unfortunately, I've been using Vim long enough to remember when Q was replaced with the young upstart gq. Oh, how our little fingers suffered in the olden days.
@chrisbra, if we're going to go this route then we should consider removing all use of formatprg in ftplugins rather than just for Rust. By definition they're not going to work like the internal formatter in all cases. Off the top of my head, Go, Modula-3, and maybe Awk, don't work like the default formatter in this specific context either.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
D. Ben Knoble
benknoble left a comment (vim/vim#18650)@chrisbra, if we're going to go this route then we should consider removing all use of
formatprgin ftplugins rather than just for Rust. By definition they're not going to work like the internal formatter in all cases. Off the top of my head, Go, Modula-3, and maybe Awk, don't work like the default formatter in this specific context either.
Which will surely bring out other complaints ;) We cannot please everyone.
Perhaps we need to spell out what our backward compatible guarantee covers for filetype support? We obviously don't control the behavior of external programs, and I've always considered filetypes a grey area where incompatible changes were permissible to an extent (though of course I've so far not upstreamed a potentially controversial change related to Racket filetypes).
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18650)Tough choice. In this case I reverted it mainly because it was the second or third complaint about this change. Removing some settings that users have already are used to, will probably cause additional complaints. My personal preference is to try to avoid more disruption here.
It sounds like we are going into a direction about the general usefuleness of formatter plugins and what should and should not belong into a filetype plugin. I guess we can't please everybody (e.g. what should we try to provide as a convenience setup vs. what should each user configure on their own).
Perhaps we should try to finish up the proposed formatter PR. But that would still need every user to configure his favorite formatter for the lanuage to be used.
Regarding the backwards guarantee of filetype plugins, that's also controversial, as we are often not upstream. However we can at least try to document some rules for filetypes we maintain here of how we would like this to happen. (It's gonna be a nightmare to keep documentating those changes for runtime file adjustments however).
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
dkearns left a comment (vim/vim#18650)I strongly disagree with development being user complaint driven. Users motivated by minor inconveniences to their personal workflow aren't a very good metric.
I've always found the great weakness in the runtime file distribution is the total lack of consistency in approach. As a user, I find an undesirable feature or setting consistently applied much easier to deal with day to day than every approach applied inconsistently across the board.
Perhaps we should try to finish up the proposed formatter PR. But that would still need every user to configure his favorite formatter for the lanuage to be used.
Yes, I think that is a good idea. Again, as canonical formatters certainly aren't always available, if the rule is that these need to be manually configured for every filetype I'm a satisfied user. I've never been that comfortable with :compiler being set in ftplugins.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18650)I see that makes sense, thanks for pushing for consistency here. In that case, you can create a PR for removing those formatters as well.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
D. Ben Knoble
benknoble left a comment (vim/vim#18650)In that case, I think you'd want to remove raco fmt from the Racket ftplugin, though upstream will continue to have it (and I may have to adjust my sync process to avoid letting it slip back in), unless you are content to leave downstream inclusions alone?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yee Cheng Chin
ychin left a comment (vim/vim#18650)I think a reasonable thing to discuss here is: how should a user configure these optional file type bindings? Are we asking each file type to optionally enable their additional bindings via something like no_rust_maps? Each file type will get more optionality this way but it's somewhat annoying if you edit multiple file types and have to look up every single one of them. You may also not want everything. Sometimes you really just want a formatter support.
Alternatively, we could have a general let g:enable_filetype_formatter=1 type setting in vimrc that will turn on all formatprg / formatexpr settings in all file types by convention. The reason why this may make sense at least within formatting's context is that a user who is aware of this should already know that gq is specifically designed for using these options, and gw is for using Vim's own formatter. A user (like this issue's filer) who doesn't know it will not have this turned on but an informed user probably just wants gq to "just works" on every file type that knows how to format. Hell, you can even make this part of defaults.vim. I guess seems from the discussion sometimes the default formatter isn't known so it complicates things.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.