Skip to content

Commit 9d3926d

Browse files
committed
Switch to using buildifier's -path option
Buildifier offers a -path command line option: > Buildifier's reformatting depends in part on the path to the file > relative to the workspace directory. Normally buildifier deduces > that path from the file names given, but the path can be given > explicitly with the -path argument. This is especially useful when > reformatting standard input, or in scripts that reformat a temporary > copy of a file. This lets us drop our `-type`-guessing logic. For the moment, we rely on the buffer filename being "relative to the workspace directory", which it generally is, but in a future change, we should introduce logic that will locate the WORKSPACE file and adjusts the filename relative to that directory. This could be complicated based on the working directory in which `buildifier` is executed, so a little more research is necessary to implement that logic correctly.
1 parent ed7f4de commit 9d3926d

File tree

2 files changed

+8
-33
lines changed

2 files changed

+8
-33
lines changed

autoload/ale/fixers/buildifier.vim

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,9 @@ endfunction
1414
function! ale#fixers#buildifier#Fix(buffer) abort
1515
let l:executable = ale#Escape(ale#fixers#buildifier#GetExecutable(a:buffer))
1616
let l:options = ale#Var(a:buffer, 'bazel_buildifier_options')
17-
let l:filename = fnamemodify(bufname(a:buffer), ':t')
17+
let l:filename = ale#Escape(bufname(a:buffer))
1818

19-
let l:command = l:executable . ' -mode fix -lint fix'
20-
21-
" Attempt to guess the file type based on the filename. buildifier itself
22-
" usually does this based on the filenames provided on the command line,
23-
" but because we're piping our buffer via stdin, we do this manually.
24-
if l:filename =~? 'WORKSPACE'
25-
let l:command .= ' -type workspace'
26-
elseif l:filename =~? 'BUILD'
27-
let l:command .= ' -type build'
28-
elseif l:filename =~? '.bzl$'
29-
let l:command .= ' -type bzl'
30-
endif
19+
let l:command = l:executable . ' -mode fix -lint fix -path ' . l:filename
3120

3221
if l:options isnot# ''
3322
let l:command .= ' ' . l:options

test/fixers/test_buildifier_fixer_callback.vader

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ Execute(The buildifier callback should return the correct default values):
1111
AssertFixer
1212
\ {
1313
\ 'command': ale#Escape(g:ale_bazel_buildifier_executable)
14-
\ . ' -mode fix -lint fix -type workspace -'
14+
\ . ' -mode fix -lint fix -path '
15+
\ . ale#test#GetFilename('bazel_paths/WORKSPACE')
16+
\ . ' -'
1517
\ }
1618

1719
Execute(The buildifier callback should include any additional options):
@@ -21,23 +23,7 @@ Execute(The buildifier callback should include any additional options):
2123
AssertFixer
2224
\ {
2325
\ 'command': ale#Escape(g:ale_bazel_buildifier_executable)
24-
\ . ' -mode fix -lint fix -type workspace --some-option -',
25-
\ }
26-
27-
Execute(The buildifier callback should recognize BUILD files):
28-
call ale#test#SetFilename('bazel_paths/BUILD')
29-
30-
AssertFixer
31-
\ {
32-
\ 'command': ale#Escape(g:ale_bazel_buildifier_executable)
33-
\ . ' -mode fix -lint fix -type build -'
34-
\ }
35-
36-
Execute(The buildifier callback should recognize .bzl files):
37-
call ale#test#SetFilename('bazel_paths/defs.bzl')
38-
39-
AssertFixer
40-
\ {
41-
\ 'command': ale#Escape(g:ale_bazel_buildifier_executable)
42-
\ . ' -mode fix -lint fix -type bzl -'
26+
\ . ' -mode fix -lint fix -path '
27+
\ . ale#test#GetFilename('bazel_paths/WORKSPACE')
28+
\ . ' --some-option -'
4329
\ }

0 commit comments

Comments
 (0)