Skip to content

Commit

Permalink
code_action: Fix EOL at EOF corner cases while performing no changes
Browse files Browse the repository at this point in the history
Instead of always removing the trailing empty line and hoping it wasn't
supposed to be there, only remove it when we know ale#util#Writefile
will add a trailing newline, and additionally tell ale#util#Writefile to
not add that newline if any change explicitly deleted that newline.

(Unfortunately with a:should_save=0, adding a trailing newline that
wasn't there results in noeol and empty line at end of buffer, because
for some reason setbufvar(l:buffer, '&eol', 1) doesn't work in buffers
that started as noeol. This is likely a bug in vim itself.)

Includes a minor fix that prevents invoking ale#util#SetBufferContents
without a buffer. Not sure whether that might really happen anywhere.

(test_code_action_python.vader depends on two incorrect behaviours
cancelling each other, so it's temporarily adjusted to pass)
  • Loading branch information
liskin committed Dec 5, 2020
1 parent a4b7069 commit 53f8856
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 8 deletions.
19 changes: 17 additions & 2 deletions autoload/ale/code_action.vim
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ function! ale#code_action#ApplyChanges(filename, changes, should_save) abort

if l:buffer > 0
let l:lines = getbufline(l:buffer, 1, '$')

" Add empty line if there's trailing newline, like readfile() does.
if getbufvar(l:buffer, '&eol')
let l:lines += ['']
endif
else
let l:lines = readfile(a:filename, 'b')
endif
Expand Down Expand Up @@ -131,11 +136,21 @@ function! ale#code_action#ApplyChanges(filename, changes, should_save) abort
endif
endfor

if l:lines[-1] is# ''
if l:buffer > 0
" Make sure ale#util#{Writefile,SetBufferContents} add trailing
" newline if and only if it should be added.
if l:lines[-1] is# '' && getbufvar(l:buffer, '&eol')
call remove(l:lines, -1)
else
call setbufvar(l:buffer, '&eol', 0)
endif
elseif exists('+fixeol') && &fixeol && l:lines[-1] is# ''
" Not in buffer, ale#util#Writefile can't check &eol and always adds
" newline if &fixeol: remove to prevent double trailing newline.
call remove(l:lines, -1)
endif

if a:should_save
if a:should_save || l:buffer < 0
call ale#util#Writefile(l:buffer, l:lines, a:filename)
else
call ale#util#SetBufferContents(l:buffer, l:lines)
Expand Down
4 changes: 0 additions & 4 deletions test/test_code_action.vader
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ Before:
Save g:ale_enabled
let g:ale_enabled = 0

" Enable fix end-of-line as tests below expect that
Save &fixeol
set fixeol

let g:file1 = tempname()
let g:file2 = tempname()
let g:test = {}
Expand Down
75 changes: 75 additions & 0 deletions test/test_code_action_corner_cases.vader
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
Before:
Save &fixeol
set nofixeol

Save &fileformats
set fileformats=unix

" two files, one accessed through a buffer, the other using write/readfile only
let g:files = [tempname(), tempname()]

function! TestChanges(contents, changes, mode) abort
let l:file = g:files[a:mode is 'file' ? 0 : 1]
call writefile(split(a:contents, '\n', 1), l:file, 'bS')
if a:mode isnot 'file'
execute 'edit ' . l:file
endif
call ale#code_action#ApplyChanges(l:file, a:changes, a:mode isnot 'buffer')
if a:mode is 'buffer'
execute 'write ' . l:file
endif
return join(readfile(l:file, 'b'), "\n")
endfunction!

function! MkPos(line, offset) abort
return {'line': a:line, 'offset': a:offset}
endfunction!

function! MkDelete(start, end) abort
return {'start': a:start, 'end': a:end, 'newText': ''}
endfunction!

After:
for g:file in g:files
if bufnr(g:file) != -1
execute ':bp! | :bd! ' . bufnr(g:file)
endif
if filereadable(g:file)
call delete(g:file)
endif
endfor
unlet! g:files g:file

unlet! g:mode

delfunction TestChanges
delfunction MkPos
delfunction MkDelete

Restore

Execute(Preserve (no)eol at eof):
for g:mode in ['save', 'file', 'buffer']
Log g:mode
AssertEqual "noeol", TestChanges("noeol", [], g:mode)
AssertEqual "eol\n", TestChanges("eol\n", [], g:mode)
AssertEqual "eols\n\n", TestChanges("eols\n\n", [], g:mode)
endfor

" there doesn't seem to be a way to tell if a buffer is empty or contains one
" empty line :-(
AssertEqual "", TestChanges("", [], 'file')

Execute(Respect fixeol):
set fixeol
for g:mode in ['save', 'file', 'buffer']
Log g:mode
AssertEqual "noeol\n", TestChanges("noeol", [], g:mode)
AssertEqual "eol\n", TestChanges("eol\n", [], g:mode)
endfor

Execute(Del eol at eof):
for g:mode in ['save', 'file', 'buffer']
Log g:mode
AssertEqual "deleol", TestChanges("deleolx\n", [MkDelete(MkPos(1, 7), MkPos(2, 1))], g:mode)
endfor
4 changes: 2 additions & 2 deletions test/test_code_action_python.vader
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Execute():
let g:changes = [
\ {'end': {'offset': 7, 'line': 1}, 'newText': 'func_qtffgsv', 'start': {'offset': 5, 'line': 1}},
\ {'end': {'offset': 9, 'line': 1}, 'newText': '', 'start': {'offset': 8, 'line': 1}},
\ {'end': {'offset': 15, 'line': 3}, 'newText': " return c\n\n\ndef main():\n c = func_qtffgsvi()\n", 'start': {'offset': 15, 'line': 3}}
\ {'end': {'offset': 1, 'line': 4}, 'newText': " return c\n\n\ndef main():\n c = func_qtffgsvi()\n", 'start': {'offset': 15, 'line': 3}}
\]

call ale#code_action#ApplyChanges(expand('%:p'), g:changes, 0)
Expand Down Expand Up @@ -37,7 +37,7 @@ Execute():
let g:changes = [
\ {'end': {'offset': 16, 'line': 2}, 'newText': "\n\ndef func_ivlpdpao(f):\n exif = exifread.process_file(f)\n dt = str(exif['Image DateTime'])\n date = dt[:10].replace(':', '-')\n return date\n", 'start': {'offset': 16, 'line': 2}},
\ {'end': {'offset': 32, 'line': 6}, 'newText': 'date = func', 'start': {'offset': 9, 'line': 6}},
\ {'end': {'offset': 42, 'line': 8}, 'newText': "ivlpdpao(f)\n", 'start': {'offset': 33, 'line': 6}}
\ {'end': {'offset': 1, 'line': 9}, 'newText': "ivlpdpao(f)\n", 'start': {'offset': 33, 'line': 6}}
\]

call ale#code_action#ApplyChanges(expand('%:p'), g:changes, 0)
Expand Down

0 comments on commit 53f8856

Please sign in to comment.