Skip to content

Some improvements to the Vim Cargo compiler file. #17513

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.

Sign up for GitHub

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
wants to merge 4 commits into from
Closed

Some improvements to the Vim Cargo compiler file. #17513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1e2e2ac
Some improvements to the Cargo compiler file.
Sep 24, 2014
83f6a29
Use rustc's errorformat and add option to specify permanent parameters.
Oct 7, 2014
0207e25
Updates based on kballard's feedback.
Oct 24, 2014
75e4d95
Added check for absolute file path, removed hard tab, and added docum…
Nov 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 additions & 15 deletions src/etc/vim/compiler/cargo.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,35 @@
" Vim compiler file
" Compiler: Cargo Compiler
" Maintainer: Damien Radtke <[email protected]>
" Latest Revision: 2014 Sep 18
" Latest Revision: 2014 Sep 24

if exists("current_compiler")
if exists('current_compiler')
finish
endif
runtime compiler/rustc.vim
let current_compiler = "cargo"

if exists(":CompilerSet") != 2
if exists(':CompilerSet') != 2
command -nargs=* CompilerSet setlocal <args>
endif

CompilerSet errorformat&
CompilerSet makeprg=cargo\ $*
if exists('g:cargo_makeprg_params')
execute 'CompilerSet makeprg=cargo\ '.escape(g:cargo_makeprg_params, ' \|"').'\ $*'
else
CompilerSet makeprg=cargo\ $*
endif

" Allow a configurable global Cargo.toml name. This makes it easy to
" support variations like 'cargo.toml'.
if !exists('g:cargo_toml_name')
let g:cargo_toml_name = 'Cargo.toml'
endif
let s:cargo_manifest_name = get(g:, 'cargo_manifest_name', 'Cargo.toml')

let s:toml_dir = fnamemodify(findfile(g:cargo_toml_name, '.;'), ':p:h').'/'
function! s:is_absolute(path)
return a:path[0] == '/' || a:path =~ '[A-Z]\+:'
endfunction

if s:toml_dir != ''
let s:local_manifest = findfile(s:cargo_manifest_name, '.;')
if s:local_manifest != ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if condition is doing nothing. The fnamemodify() will expand an empty string to the cwd, and even if that didn't, the .'/' will also guarantee the string isn't empty.

I'd recommend instead doing something like

let s:local_manifest = findfile(s:cargo_manifest_name, '.;')

if s:local_manifest != ''
    let s:local_manifest = fnamemodify(s:local_manifest, ':p:h').'/'
    ...

(s:cargo_manifest_name coming from the previous suggested change)

let s:local_manifest = fnamemodify(s:local_manifest, ':p:h').'/'
augroup cargo
au!
au QuickfixCmdPost make call s:FixPaths()
Expand All @@ -33,15 +39,25 @@ if s:toml_dir != ''
" to be relative to the current directory instead of Cargo.toml.
function! s:FixPaths()
let qflist = getqflist()
let manifest = s:local_manifest
for qf in qflist
if !qf['valid']
if !qf.valid
let m = matchlist(qf.text, '(file://\(.*\))$')
if !empty(m)
let manifest = m[1].'/'
" Manually strip another slash if needed; usually just an
" issue on Windows.
if manifest =~ '^/[A-Z]\+:/'
let manifest = manifest[1:]
endif
endif
continue
endif
let filename = bufname(qf['bufnr'])
if stridx(filename, s:toml_dir) == -1
let filename = s:toml_dir.filename
let filename = bufname(qf.bufnr)
if s:is_absolute(filename)
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these two lines for? Can't we rely on the output from Cargo being relative to the manifest? The problem with having this conditional is if I have a directory structure like

Cargo.toml
foo.rs
bar/
    foo.rs

and I am in bar/ when I run :make, an error in the top-level foo.rs will show up with the name foo.rs, and leaving that intact will incorrectly interpret the error as being in bar/foo.rs.

I think instead what needs to be done is the filename needs to be tested to see if it's absolute. If it is absolute, then it can be left alone, but if it's relative then it needs to be appended to manifest as happens a few lines down.

endif
let qf['filename'] = simplify(s:toml_dir.bufname(qf['bufnr']))
let qf.filename = simplify(manifest.filename)
call remove(qf, 'bufnr')
endfor
call setqflist(qflist, 'r')
Expand Down
7 changes: 7 additions & 0 deletions src/etc/vim/doc/rust.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ g:ftplugin_rust_source_path~
let g:ftplugin_rust_source_path = $HOME.'/dev/rust'
<

*g:cargo_manifest_name*
g:cargo_manifest_name~
Set this option to the name of the manifest file for your projects. If
not specified it defaults to 'Cargo.toml' : >
let g:cargo_manifest_name = 'Cargo.toml'
<

==============================================================================
COMMANDS *rust-commands*

Expand Down