Skip to content

Use 'setfiletype' to avoid load 'ftplugin/rust.vim' twice #301

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

Merged
merged 9 commits into from
Jan 11, 2019

Conversation

dbger
Copy link
Contributor

@dbger dbger commented Jan 10, 2019

au BufNewFile,BufRead *.rs setf rust in $VIMRUNTIME/filetype.vim had set the rust filetye, autocmd BufRead,BufNewFile *.rs set filetype=rust will overrule it.
This cause ftplugin/rust.vim will be loaded twice when the first is not loaded yet.

@rust-highfive
Copy link

r? @chris-morgan

(rust_highfive has picked a reviewer for you, use r? to override)

@chris-morgan
Copy link
Member

I am startled to see set filetype= rather than setf , because the latter is definitely the right way to do it; looks like it was added the wrong way right back at the start, seven years ago: 7d6eef9.

I don’t think I’ve ever seen setfiletype spelled in full before. setf is definitely the idiomatic spelling, change it to that then I’ll give r+.

Use the definitely the idiomatic spelling `setf`
@dbger
Copy link
Contributor Author

dbger commented Jan 11, 2019

I am startled to see set filetype= rather than setf , because the latter is definitely the right way to do it; looks like it was added the wrong way right back at the start, seven years ago: 7d6eef9.

I don’t think I’ve ever seen setfiletype spelled in full before. setf is definitely the idiomatic spelling, change it to that then I’ll give r+.

It's done.

@chris-morgan chris-morgan merged commit 12f549f into rust-lang:master Jan 11, 2019
alok added a commit to alok/rust.vim that referenced this pull request Feb 5, 2019
* remotes/original/master: (423 commits)
  Use 'setfiletype' to avoid load 'ftplugin/rust.vim' twice  (rust-lang#301)
  avoid setting `isfname`
  allow 'pub use'
  Set `include` and improve `includeexpr`
  Revert "BufWritePre should be <buffer>"
  Recognize edition2015 and edition2018 doc test specifiers
  Revert "RustTest: specify module name to run exact one test case"
  RustTest: specify module name to run exact one test case
  RustTest: use :terminal for running tests
  ftplugin/rust.vim: Better to narrow augroup scope
  rustfmt: remove incorrect version check
  rustfmt: simplify regex tests
  Improve 'async' contextual keyword highlighting (rust-lang#290)
  RustFmt: fix non-existant temp file
  Add new keywords (rust-lang#282)
  Add SyntasticInfo if exists to the output of RustInfo
  Check that Syntastic is new enough
  Update rust.txt
  Add `cargo#guessrun` function & `Cgrun` command
  Fix RunRustfmt() for Vim 7.x
  ...
da-x pushed a commit that referenced this pull request May 19, 2019
- `:set filetype` forces a filetype.
- `:setfiletype` only sets a filetype if it didn't already happen earlier due to
the same event.

Current versions of Vim and Neovim have a `$VIMRUNTIME/filetype.vim` that sets
the `rust` filetype for all files with an `.rs` extension.

This plugin did that with `set filetype=rust` as well. But now, with the
filetype getting set twice, `ftplugin/rust/*` was sourced twice as well.

This got fixed in #301 by changing
`set filetype=rust` to `setfiletype rust`.

But that fix introduced a regression for all older Vim versions. There is still
a lot of Vim 7.4 around and Debian stable still provides Nvim 0.1.7, both being
very old versions.

The `$VIMRUNTIME/filetype.vim` of these old versions set the filetype to
`hercules` for all `.rs` files via `setfiletype hercules`.

Now, usually (it also depends on how plugins gets sourced) `filetype.vim` gets
sourced _before_ any `ftdetect/*` files from plugins. And since autocmds are
processed in order, Vim would first do `setfiletype hercules` and so any
subsequent `setfiletype rust` fails.

The obvious solution: Force the `rust` filetype for all `.rs` extensions unless
the filetype is already set to `rust`.

So, we have the filetype setting working again for older versions and also avoid
setting the filetype twice.

Fixes #306
Fixes #318
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants