From cbaa22e38c8884d8b0394aa248cd6d9f46998a53 Mon Sep 17 00:00:00 2001 From: Andy Stewart Date: Fri, 16 Aug 2019 16:21:06 +0100 Subject: [PATCH] Use sign functions, groups, and priority, where available. On Vims that support it, signs are placed in the "gitgutter" group with a priority set by g:gitgutter_sign_priority. Closes #544. Closes #576. Closes #627. --- autoload/gitgutter/sign.vim | 104 ++++++++++++++------ plugin/gitgutter.vim | 5 +- test/test_gitgutter.vim | 190 +++++++++++++++++++----------------- 3 files changed, 182 insertions(+), 117 deletions(-) diff --git a/autoload/gitgutter/sign.vim b/autoload/gitgutter/sign.vim index fb604a4..1ee305a 100644 --- a/autoload/gitgutter/sign.vim +++ b/autoload/gitgutter/sign.vim @@ -1,8 +1,4 @@ -" Vim doesn't namespace sign ids so every plugin shares the same -" namespace. Sign ids are simply integers so to avoid clashes with other -" signs we guess at a clear run. -" -" Note also we currently never reset s:next_sign_id. +" For older Vims without sign_place() the plugin has to manaage the sign ids. let s:first_sign_id = 3000 let s:next_sign_id = s:first_sign_id " Remove-all-signs optimisation requires Vim 7.3.596+. @@ -40,6 +36,12 @@ endfunction " Removes gitgutter's signs from the buffer being processed. function! gitgutter#sign#clear_signs(bufnr) abort + if exists('*sign_unplace') + call sign_unplace('gitgutter', {'buffer': a:bufnr}) + return + endif + + call s:find_current_signs(a:bufnr) let sign_ids = map(values(gitgutter#utility#getbufvar(a:bufnr, 'gitgutter_signs')), 'v:val.id') @@ -53,6 +55,35 @@ endfunction " modified_lines: list of [, ] " where name = 'added|removed|modified|modified_removed' function! gitgutter#sign#update_signs(bufnr, modified_lines) abort + if exists('*sign_unplace') + " Vim is (hopefully) now quick enough to remove all signs then place new ones. + call sign_unplace('gitgutter', {'buffer': a:bufnr}) + + let modified_lines = s:handle_double_hunk(a:modified_lines) + let signs = map(copy(modified_lines), '{'. + \ '"buffer": a:bufnr,'. + \ '"group": "gitgutter",'. + \ '"name": s:highlight_name_for_change(v:val[1]),'. + \ '"lnum": v:val[0],'. + \ '"priority": g:gitgutter_sign_priority'. + \ '}') + + if exists('*sign_placelist') + call sign_placelist(signs) + return + endif + + for sign in signs + call sign_place(0, sign.group, sign.name, sign.buffer, {'lnum': sign.lnum, 'priority': sign.priority}) + endfor + return + endif + + + " Derive a delta between the current signs and the ones we want. + " Remove signs from lines that no longer need a sign. + " Upsert the remaining signs. + call s:find_current_signs(a:bufnr) let new_gitgutter_signs_line_numbers = map(copy(a:modified_lines), 'v:val[0]') @@ -74,28 +105,40 @@ function! s:find_current_signs(bufnr) abort let other_signs = [] " [ signs - silent execute "sign place buffer=" . a:bufnr - redir END + if exists('*getbufinfo') + let bufinfo = getbufinfo(a:bufnr)[0] + let signs = has_key(bufinfo, 'signs') ? bufinfo.signs : [] + else + let signs = [] - for sign_line in filter(split(signs, '\n')[2:], 'v:val =~# "="') - " Typical sign line: line=88 id=1234 name=GitGutterLineAdded - " We assume splitting is faster than a regexp. - let components = split(sign_line) - let name = split(components[2], '=')[1] - let line_number = str2nr(split(components[0], '=')[1]) - if name =~# 'GitGutter' - let id = str2nr(split(components[1], '=')[1]) + redir => signlines + silent execute "sign place buffer=" . a:bufnr + redir END + + for signline in filter(split(signlines, '\n')[2:], 'v:val =~# "="') + " Typical sign line before v8.1.0614: line=88 id=1234 name=GitGutterLineAdded + " We assume splitting is faster than a regexp. + let components = split(signline) + call add(signs, { + \ 'lnum': str2nr(split(components[0], '=')[1]), + \ 'id': str2nr(split(components[1], '=')[1]), + \ 'name': split(components[2], '=')[1] + \ }) + endfor + endif + + for sign in signs + if sign.name =~# 'GitGutter' " Remove orphaned signs (signs placed on lines which have been deleted). " (When a line is deleted its sign lingers. Subsequent lines' signs' " line numbers are decremented appropriately.) - if has_key(gitgutter_signs, line_number) - execute "sign unplace" gitgutter_signs[line_number].id + if has_key(gitgutter_signs, sign.lnum) + execute "sign unplace" gitgutter_signs[sign.lnum].id endif - let gitgutter_signs[line_number] = {'id': id, 'name': name} + let gitgutter_signs[sign.lnum] = {'id': sign.id, 'name': sign.name} else if !g:gitgutter_sign_allow_clobber - call add(other_signs, line_number) + call add(other_signs, sign.lnum) endif endif endfor @@ -142,14 +185,7 @@ function! s:upsert_new_gitgutter_signs(bufnr, modified_lines) abort endif let old_gitgutter_signs = gitgutter#utility#getbufvar(a:bufnr, 'gitgutter_signs') - " Handle special case where the first line is the site of two hunks: - " lines deleted above at the start of the file, and lines deleted - " immediately below. - if a:modified_lines[0:1] == [[1, 'removed_first_line'], [1, 'removed']] - let modified_lines = [[1, 'removed_above_and_below']] + a:modified_lines[2:] - else - let modified_lines = a:modified_lines - endif + let modified_lines = s:handle_double_hunk(a:modified_lines) for line in modified_lines let line_number = line[0] " @@ -170,6 +206,18 @@ function! s:upsert_new_gitgutter_signs(bufnr, modified_lines) abort endfunction +" Handle special case where the first line is the site of two hunks: +" lines deleted above at the start of the file, and lines deleted +" immediately below. +function! s:handle_double_hunk(modified_lines) + if a:modified_lines[0:1] == [[1, 'removed_first_line'], [1, 'removed']] + return [[1, 'removed_above_and_below']] + a:modified_lines[2:] + endif + + return a:modified_lines +endfunction + + function! s:next_sign_id() abort let next_id = s:next_sign_id let s:next_sign_id += 1 diff --git a/plugin/gitgutter.vim b/plugin/gitgutter.vim index 7d04483..0901f0b 100644 --- a/plugin/gitgutter.vim +++ b/plugin/gitgutter.vim @@ -28,7 +28,10 @@ call s:set('g:gitgutter_max_signs', 500) call s:set('g:gitgutter_signs', 1) call s:set('g:gitgutter_highlight_lines', 0) call s:set('g:gitgutter_highlight_linenrs', 0) -if has('nvim-0.4.0') && !exists('g:gitgutter_sign_allow_clobber') +call s:set('g:gitgutter_sign_priority', 10) +" Nvim 0.4.0 has an expanding sign column +" The sign_place() function supports sign priority. +if (has('nvim-0.4.0') || exists('*sign_place')) && !exists('g:gitgutter_sign_allow_clobber') let g:gitgutter_sign_allow_clobber = 1 endif call s:set('g:gitgutter_sign_allow_clobber', 0) diff --git a/test/test_gitgutter.vim b/test/test_gitgutter.vim index 9130105..58c0565 100644 --- a/test/test_gitgutter.vim +++ b/test/test_gitgutter.vim @@ -6,21 +6,27 @@ let s:bufnr = bufnr('') " Helpers " -function s:signs(filename) - redir => signs - silent execute 'sign place' - redir END +" Ignores unexpected keys. +" +" expected - list of signs +function s:assert_signs(expected, filename) + if empty(a:expected) + call assert_equal(a:expected, []) + return + endif - let signs = split(signs, '\n') + let expected_keys = keys(a:expected[0]) + let actual = sign_getplaced(a:filename, {'group': 'gitgutter'})[0].signs - " filter out signs for this test file - " assumes a:filename's signs are last set listed - let i = index(signs, 'Signs for '.a:filename.':') - let signs = (i > -1 ? signs[i+1:] : []) + for sign in actual + for k in keys(sign) + if index(expected_keys, k) == -1 + call remove(sign, k) + endif + endfor + endfor - call map(signs, {_, v -> substitute(v, ' ', '', '')}) - - return signs + call assert_equal(a:expected, actual) endfunction function s:git_diff() @@ -71,8 +77,8 @@ function Test_add_lines() normal ggo* call s:trigger_gitgutter() - let expected = ["line=2 id=3000 name=GitGutterLineAdded priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 2, 'name': 'GitGutterLineAdded', 'group': 'gitgutter', 'priority': 10}] + call s:assert_signs(expected, 'fixture.txt') endfunction @@ -83,8 +89,8 @@ function Test_add_lines_fish() normal ggo* call s:trigger_gitgutter() - let expected = ["line=2 id=3000 name=GitGutterLineAdded priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 2, 'name': 'GitGutterLineAdded'}] + call s:assert_signs(expected, 'fixture.txt') let &shell = _shell endfunction @@ -94,8 +100,8 @@ function Test_modify_lines() normal ggi* call s:trigger_gitgutter() - let expected = ["line=1 id=3000 name=GitGutterLineModified priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 1, 'name': 'GitGutterLineModified'}] + call s:assert_signs(expected, 'fixture.txt') endfunction @@ -103,8 +109,8 @@ function Test_remove_lines() execute '5d' call s:trigger_gitgutter() - let expected = ["line=4 id=3000 name=GitGutterLineRemoved priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 4, 'name': 'GitGutterLineRemoved'}] + call s:assert_signs(expected, 'fixture.txt') endfunction @@ -112,8 +118,20 @@ function Test_remove_first_lines() execute '1d' call s:trigger_gitgutter() - let expected = ["line=1 id=3000 name=GitGutterLineRemovedFirstLine priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 1, 'name': 'GitGutterLineRemovedFirstLine'}] + call s:assert_signs(expected, 'fixture.txt') +endfunction + + +function Test_priority() + let g:gitgutter_sign_priority = 5 + + execute '1d' + call s:trigger_gitgutter() + + call s:assert_signs([{'priority': 5}], 'fixture.txt') + + let g:gitgutter_sign_priority = 10 endfunction @@ -122,8 +140,8 @@ function Test_overlapping_hunks() execute '1d' call s:trigger_gitgutter() - let expected = ["line=1 id=3000 name=GitGutterLineRemovedAboveAndBelow priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 1, 'name': 'GitGutterLineRemovedAboveAndBelow'}] + call s:assert_signs(expected, 'fixture.txt') endfunction @@ -132,8 +150,8 @@ function Test_edit_file_with_same_name_as_a_branch() call system('git checkout -b fixture.txt') call s:trigger_gitgutter() - let expected = ["line=5 id=3000 name=GitGutterLineModified priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 5, 'name': 'GitGutterLineModified'}] + call s:assert_signs(expected, 'fixture.txt') endfunction @@ -144,8 +162,8 @@ function Test_file_added_to_git() normal ihello call s:trigger_gitgutter() - let expected = ["line=1 id=3000 name=GitGutterLineAdded priority=10"] - call assert_equal(expected, s:signs('fileAddedToGit.tmp')) + let expected = [{'lnum': 1, 'name': 'GitGutterLineAdded'}] + call s:assert_signs(expected, 'fileAddedToGit.tmp') endfunction @@ -156,10 +174,10 @@ function Test_filename_with_equals() call s:trigger_gitgutter() let expected = [ - \ 'line=1 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=2 id=3001 name=GitGutterLineAdded priority=10' + \ {'lnum': 1, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 2, 'name': 'GitGutterLineAdded'} \ ] - call assert_equal(expected, s:signs('=fixture=.txt')) + call s:assert_signs(expected, '=fixture=.txt') endfunction @@ -170,10 +188,10 @@ function Test_filename_with_square_brackets() call s:trigger_gitgutter() let expected = [ - \ 'line=1 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=2 id=3001 name=GitGutterLineAdded priority=10' + \ {'lnum': 1, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 2, 'name': 'GitGutterLineAdded'} \ ] - call assert_equal(expected, s:signs('fix[tu]re.txt')) + call s:assert_signs(expected, 'fix[tu]re.txt') endfunction @@ -184,10 +202,10 @@ function Test_filename_leading_dash() call s:trigger_gitgutter() let expected = [ - \ 'line=1 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=2 id=3001 name=GitGutterLineAdded priority=10' + \ {'lnum': 1, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 2, 'name': 'GitGutterLineAdded'} \ ] - call assert_equal(expected, s:signs('-fixture.txt')) + call s:assert_signs(expected, '-fixture.txt') endfunction @@ -198,10 +216,10 @@ function Test_filename_umlaut() call s:trigger_gitgutter() let expected = [ - \ 'line=1 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=2 id=3001 name=GitGutterLineAdded priority=10' + \ {'lnum': 1, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 2, 'name': 'GitGutterLineAdded'} \ ] - call assert_equal(expected, s:signs('fixtüre.txt')) + call s:assert_signs(expected, 'fixtüre.txt') endfunction @@ -213,8 +231,8 @@ function Test_follow_symlink() 6d call s:trigger_gitgutter() - let expected = ['line=5 id=3000 name=GitGutterLineRemoved priority=10'] - call assert_equal(expected, s:signs('symlink')) + let expected = [{'lnum': 5, 'name': 'GitGutterLineRemoved'}] + call s:assert_signs(expected, 'symlink') endfunction @@ -255,7 +273,7 @@ endfunction function Test_no_modifications() - call assert_equal([], s:signs('fixture.txt')) + call s:assert_signs([], 'fixture.txt') endfunction @@ -265,8 +283,8 @@ function Test_orphaned_signs() 6d call s:trigger_gitgutter() - let expected = ['line=6 id=3001 name=GitGutterLineAdded priority=10'] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 6, 'name': 'GitGutterLineAdded'}] + call s:assert_signs(expected, 'fixture.txt') endfunction @@ -275,7 +293,7 @@ function Test_untracked_file_outside_repo() call system('touch '.tmp) execute 'edit '.tmp - call assert_equal([], s:signs(tmp)) + call s:assert_signs([], tmp) endfunction @@ -286,7 +304,7 @@ function Test_untracked_file_within_repo() normal ggo* call s:trigger_gitgutter() - call assert_equal([], s:signs(tmp)) + call s:assert_signs([], tmp) call assert_equal(-2, b:gitgutter.path) call system('rm '.tmp) @@ -300,7 +318,7 @@ function Test_untracked_file_square_brackets_within_repo() normal ggo* call s:trigger_gitgutter() - call assert_equal([], s:signs(tmp)) + call s:assert_signs([], tmp) call system('rm '.tmp) endfunction @@ -310,13 +328,13 @@ function Test_hunk_outside_noop() 5 GitGutterStageHunk - call assert_equal([], s:signs('fixture.txt')) + call s:assert_signs([], 'fixture.txt') call assert_equal([], s:git_diff()) call assert_equal([], s:git_diff_staged()) GitGutterUndoHunk - call assert_equal([], s:signs('fixture.txt')) + call s:assert_signs([], 'fixture.txt') call assert_equal([], s:git_diff()) call assert_equal([], s:git_diff_staged()) endfunction @@ -332,7 +350,7 @@ function Test_hunk_stage() call assert_equal('foo', &shell) let &shell = _shell - call assert_equal([], s:signs('fixture.txt')) + call s:assert_signs([], 'fixture.txt') " Buffer is unsaved let expected = [ @@ -372,11 +390,11 @@ function Test_hunk_stage_nearby_hunk() GitGutterStageHunk let expected = [ - \ 'line=3 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=4 id=3001 name=GitGutterLineAdded priority=10', - \ 'line=5 id=3002 name=GitGutterLineAdded priority=10' + \ {'lnum': 3, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 4, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 5, 'name': 'GitGutterLineAdded'} \ ] - call assert_equal(expected, s:signs('fixture.txt')) + call s:assert_signs(expected, 'fixture.txt') " Buffer is unsaved let expected = [ @@ -422,10 +440,10 @@ function Test_hunk_stage_partial_visual_added() execute "normal 7GVj:GitGutterStageHunk\" let expected = [ - \ 'line=6 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=9 id=3001 name=GitGutterLineAdded priority=10', + \ {'lnum': 6, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 9, 'name': 'GitGutterLineAdded'}, \ ] - call assert_equal(expected, s:signs('fixture.txt')) + call s:assert_signs(expected, 'fixture.txt') let expected = [ \ 'diff --git a/fixture.txt b/fixture.txt', @@ -457,10 +475,10 @@ function Test_hunk_stage_partial_cmd_added() 7,8GitGutterStageHunk let expected = [ - \ 'line=6 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=9 id=3001 name=GitGutterLineAdded priority=10', + \ {'lnum': 6, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 9, 'name': 'GitGutterLineAdded'}, \ ] - call assert_equal(expected, s:signs('fixture.txt')) + call s:assert_signs(expected, 'fixture.txt') let expected = [ \ 'diff --git a/fixture.txt b/fixture.txt', @@ -500,10 +518,10 @@ function Test_hunk_stage_partial_preview_added() write let expected = [ - \ 'line=6 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=8 id=3002 name=GitGutterLineAdded priority=10', + \ {'lnum': 6, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 8, 'name': 'GitGutterLineAdded'}, \ ] - call assert_equal(expected, s:signs('fixture.txt')) + call s:assert_signs(expected, 'fixture.txt') let expected = [ \ 'diff --git a/fixture.txt b/fixture.txt', @@ -552,10 +570,10 @@ function Test_hunk_stage_partial_preview_added_removed() write let expected = [ - \ 'line=3 id=3004 name=GitGutterLineRemoved priority=10', - \ 'line=7 id=3003 name=GitGutterLineAdded priority=10', + \ {'lnum': 3, 'name': 'GitGutterLineRemoved'}, + \ {'lnum': 7, 'name': 'GitGutterLineAdded'}, \ ] - call assert_equal(expected, s:signs('fixture.txt')) + call s:assert_signs(expected, 'fixture.txt') let expected = [ \ 'diff --git a/fixture.txt b/fixture.txt', @@ -594,7 +612,7 @@ function Test_hunk_undo() call assert_equal('foo', &shell) let &shell = _shell - call assert_equal([], s:signs('fixture.txt')) + call s:assert_signs([], 'fixture.txt') call assert_equal([], s:git_diff()) call assert_equal([], s:git_diff_staged()) endfunction @@ -609,11 +627,11 @@ function Test_undo_nearby_hunk() call s:trigger_gitgutter() let expected = [ - \ 'line=3 id=3000 name=GitGutterLineAdded priority=10', - \ 'line=4 id=3001 name=GitGutterLineAdded priority=10', - \ 'line=5 id=3002 name=GitGutterLineAdded priority=10' + \ {'lnum': 3, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 4, 'name': 'GitGutterLineAdded'}, + \ {'lnum': 5, 'name': 'GitGutterLineAdded'} \ ] - call assert_equal(expected, s:signs('fixture.txt')) + call s:assert_signs(expected, 'fixture.txt') call assert_equal([], s:git_diff()) @@ -652,10 +670,8 @@ function Test_overlapping_hunk_op() GitGutterUndoHunk call s:trigger_gitgutter() - let expected = [ - \ 'line=2 id=3000 name=GitGutterLineRemoved priority=10', - \ ] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 2, 'name': 'GitGutterLineRemoved'}] + call s:assert_signs(expected, 'fixture.txt') " Undo lower @@ -666,10 +682,8 @@ function Test_overlapping_hunk_op() GitGutterUndoHunk call s:trigger_gitgutter() - let expected = [ - \ 'line=1 id=3000 name=GitGutterLineRemovedFirstLine priority=10', - \ ] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 1, 'name': 'GitGutterLineRemovedFirstLine'}] + call s:assert_signs(expected, 'fixture.txt') endfunction @@ -679,8 +693,8 @@ function Test_write_option() normal ggo* call s:trigger_gitgutter() - let expected = ["line=2 id=3000 name=GitGutterLineAdded priority=10"] - call assert_equal(expected, s:signs('fixture.txt')) + let expected = [{'lnum': 2, 'name': 'GitGutterLineAdded'}] + call s:assert_signs(expected, 'fixture.txt') set write endfunction @@ -692,7 +706,7 @@ function Test_inner_text_object() normal dic call s:trigger_gitgutter() - call assert_equal([], s:signs('fixture.txt')) + call s:assert_signs([], 'fixture.txt') call assert_equal(readfile('fixture.txt'), getline(1,'$')) " Excludes trailing lines @@ -710,7 +724,7 @@ function Test_around_text_object() normal dac call s:trigger_gitgutter() - call assert_equal([], s:signs('fixture.txt')) + call s:assert_signs([], 'fixture.txt') call assert_equal(readfile('fixture.txt'), getline(1,'$')) " Includes trailing lines @@ -813,7 +827,7 @@ function Test_encoding() call s:trigger_gitgutter() - call assert_equal([], s:signs('cp932.txt')) + call s:assert_signs([], 'cp932.txt') endfunction @@ -823,7 +837,7 @@ function Test_empty_file() edit empty.txt call s:trigger_gitgutter() - call assert_equal([], s:signs('empty.txt')) + call s:assert_signs([], 'empty.txt') " File consisting only of a newline @@ -831,7 +845,7 @@ function Test_empty_file() edit newline.txt call s:trigger_gitgutter() - call assert_equal([], s:signs('newline.txt')) + call s:assert_signs([], 'newline.txt') " 1 line file without newline @@ -841,7 +855,7 @@ function Test_empty_file() edit! oneline.txt call s:trigger_gitgutter() - call assert_equal([], s:signs('oneline.txt')) + call s:assert_signs([], 'oneline.txt') set eol fixeol endfunction