From 11cb53391ba0131d2aaf935b713897bd75fa1f0f Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Wed, 3 Jul 2024 11:03:58 +0100 Subject: [PATCH] refactor(git): remove some code --- lua/gitsigns/actions.lua | 2 +- lua/gitsigns/blame.lua | 2 +- lua/gitsigns/git.lua | 93 +++++++++++++----------------------- lua/gitsigns/git/blame.lua | 2 +- test/actions_spec.lua | 8 ++-- test/gitdir_watcher_spec.lua | 6 +-- test/gitsigns_spec.lua | 45 ++++++++--------- test/gs_helpers.lua | 54 +++++++++------------ test/highlights_spec.lua | 41 +--------------- test/unit_spec.lua | 71 --------------------------- 10 files changed, 84 insertions(+), 240 deletions(-) delete mode 100644 test/unit_spec.lua diff --git a/lua/gitsigns/actions.lua b/lua/gitsigns/actions.lua index c398f26bd..a7dc4bbd5 100644 --- a/lua/gitsigns/actions.lua +++ b/lua/gitsigns/actions.lua @@ -1019,7 +1019,7 @@ M.blame_line = async.create(1, function(opts) local blame_linespec = create_blame_fmt(is_committed, opts.full) if is_committed and opts.full then - local body = bcache.git_obj:command( + local body = bcache.git_obj.repo:command( { 'show', '-s', '--format=%B', result.sha }, { text = true } ) diff --git a/lua/gitsigns/blame.lua b/lua/gitsigns/blame.lua index 0d6d76631..c2f8b70a3 100644 --- a/lua/gitsigns/blame.lua +++ b/lua/gitsigns/blame.lua @@ -185,7 +185,7 @@ end local show_commit = async.create(3, function(win, open, bcache) local cursor = api.nvim_win_get_cursor(win)[1] local sha = bcache.blame[cursor].commit.sha - local res = bcache.git_obj:command({ 'show', sha }) + local res = bcache.git_obj.repo:command({ 'show', sha }) async.scheduler() local commit_buf = api.nvim_create_buf(true, true) api.nvim_buf_set_name(commit_buf, bcache:get_rev_bufname(sha, true)) diff --git a/lua/gitsigns/git.lua b/lua/gitsigns/git.lua index dac16a082..387b02cd8 100644 --- a/lua/gitsigns/git.lua +++ b/lua/gitsigns/git.lua @@ -56,7 +56,6 @@ local Repo = {} M.Repo = Repo --- @class Gitsigns.Git.JobSpec : vim.SystemOpts ---- @field command? string --- @field ignore_error? boolean --- @async @@ -67,7 +66,7 @@ local function git_command(args, spec) spec = spec or {} local cmd = { - spec.command or 'git', + 'git', '--no-pager', '--no-optional-locks', '--literal-pathspecs', @@ -85,15 +84,17 @@ local function git_command(args, spec) --- @type vim.SystemCompleted local obj = asystem(cmd, spec) - local stdout = obj.stdout - local stderr = obj.stderr if not spec.ignore_error and obj.code > 0 then - local cmd_str = table.concat(cmd, ' ') - log.eprintf("Received exit code %d when running command\n'%s':\n%s", obj.code, cmd_str, stderr) + log.eprintf( + "Received exit code %d when running command\n'%s':\n%s", + obj.code, + table.concat(cmd, ' '), + obj.stderr + ) end - local stdout_lines = vim.split(stdout or '', '\n') + local stdout_lines = vim.split(obj.stdout or '', '\n') if spec.text then -- If stdout ends with a newline, then remove the final empty string after @@ -110,11 +111,11 @@ local function git_command(args, spec) end end - if stderr == '' then - stderr = nil + if obj.stderr == '' then + obj.stderr = nil end - return stdout_lines, stderr + return stdout_lines, obj.stderr end --- @async @@ -142,18 +143,16 @@ function M.diff(file_cmp, file_buf, indent_heuristic, diff_algo) end --- @async ---- @param gitdir string +--- @param gitdir? string --- @param head_str string --- @param cwd string ---- @param cmd? string --- @return string -local function process_abbrev_head(gitdir, head_str, cwd, cmd) +local function process_abbrev_head(gitdir, head_str, cwd) if not gitdir then return head_str end if head_str == 'HEAD' then local short_sha = git_command({ 'rev-parse', '--short', 'HEAD' }, { - command = cmd, ignore_error = true, cwd = cwd, })[1] or '' @@ -173,38 +172,26 @@ end local has_cygpath = jit and jit.os == 'Windows' and vim.fn.executable('cygpath') == 1 -local cygpath_convert ---@type fun(path: string): string - -if has_cygpath then - cygpath_convert = function(path) - --- @type vim.SystemCompleted - local obj = asystem({ 'cygpath', '-aw', path }) - return obj.stdout - end -end - ---- @param path string ---- @return string +--- @param path? string +--- @return string? local function normalize_path(path) if path and has_cygpath and not uv.fs_stat(path) then -- If on windows and path isn't recognizable as a file, try passing it -- through cygpath - path = cygpath_convert(path) + path = asystem({ 'cygpath', '-aw', path }).stdout end return path end --- @async --- @param cwd string ---- @param cmd? string --- @param gitdir? string --- @param toplevel? string --- @return Gitsigns.RepoInfo -function M.get_repo_info(cwd, cmd, gitdir, toplevel) +function M.get_repo_info(cwd, gitdir, toplevel) -- Does git rev-parse have --absolute-git-dir, added in 2.13: -- https://public-inbox.org/git/20170203024829.8071-16-szeder.dev@gmail.com/ local has_abs_gd = check_version({ 2, 13 }) - local git_dir_opt = has_abs_gd and '--absolute-git-dir' or '--git-dir' -- Wait for internal scheduler to settle before running command (#215) scheduler() @@ -222,13 +209,12 @@ function M.get_repo_info(cwd, cmd, gitdir, toplevel) vim.list_extend(args, { 'rev-parse', '--show-toplevel', - git_dir_opt, + has_abs_gd and '--absolute-git-dir' or '--git-dir', '--abbrev-ref', 'HEAD', }) local results = git_command(args, { - command = cmd, ignore_error = true, cwd = toplevel or cwd, }) @@ -243,7 +229,7 @@ function M.get_repo_info(cwd, cmd, gitdir, toplevel) return { toplevel = toplevel_r, gitdir = gitdir_r, - abbrev_head = process_abbrev_head(gitdir_r, results[3], cwd, cmd), + abbrev_head = process_abbrev_head(gitdir_r, results[3], cwd), detached = toplevel_r and gitdir_r ~= toplevel_r .. '/.git', } end @@ -261,10 +247,7 @@ function Repo:command(args, spec) spec = spec or {} spec.cwd = self.toplevel - local args1 = { - '--git-dir', - self.gitdir, - } + local args1 = { '--git-dir', self.gitdir } if self.detached then vim.list_extend(args1, { '--work-tree', self.toplevel }) @@ -310,7 +293,6 @@ function Repo:get_show_text(object, encoding) if encoding and encoding ~= 'utf-8' and iconv_supported(encoding) then for i, l in ipairs(stdout) do - --- @diagnostic disable-next-line:param-type-mismatch stdout[i] = vim.iconv(l, encoding, 'utf-8') end end @@ -331,7 +313,7 @@ end function Repo.new(dir, gitdir, toplevel) local self = setmetatable({}, { __index = Repo }) - local info = M.get_repo_info(dir, nil, gitdir, toplevel) + local info = M.get_repo_info(dir, gitdir, toplevel) for k, v in pairs(info --[[@as table]]) do @@ -348,18 +330,9 @@ end -- Git object methods -------------------------------------------------------------------------------- ---- Run git command the with the objects gitdir and toplevel ---- @param args string[] ---- @param spec? Gitsigns.Git.JobSpec ---- @return string[] stdout, string? stderr -function Obj:command(args, spec) - return self.repo:command(args, spec) -end - --- @param revision? string function Obj:update_revision(revision) - revision = util.norm_base(revision) - self.revision = revision + self.revision = util.norm_base(revision) self:update() end @@ -431,7 +404,7 @@ function Obj:file_info_index(file, silent) cmd[#cmd + 1] = file or self.file - local results, stderr = self:command(cmd, { ignore_error = true }) + local results, stderr = self.repo:command(cmd, { ignore_error = true }) if stderr and not silent then -- ignore_error for the cases when we run: @@ -478,7 +451,7 @@ end --- @param silent? boolean --- @return Gitsigns.FileInfo function Obj:file_info_tree(file, silent) - local results, stderr = self:command({ + local results, stderr = self.repo:command({ '-c', 'core.quotepath=off', 'ls-tree', @@ -548,7 +521,7 @@ local function autocmd_changed(file) end function Obj:unstage_file() - self:command({ 'reset', self.file }) + self.repo:command({ 'reset', self.file }) autocmd_changed(self.file) end @@ -598,12 +571,12 @@ local function ensure_file_in_index(obj) if not obj.object_name then -- If there is no object_name then it is not yet in the index so add it - obj:command({ 'add', '--intent-to-add', obj.file }) + obj.repo:command({ 'add', '--intent-to-add', obj.file }) else -- Update the index with the common ancestor (stage 1) which is what bcache -- stores local info = string.format('%s,%s,%s', obj.mode_bits, obj.object_name, obj.relpath) - obj:command({ 'update-index', '--add', '--cacheinfo', info }) + obj.repo:command({ 'update-index', '--add', '--cacheinfo', info }) end obj:update() @@ -612,7 +585,7 @@ end --- Stage 'lines' as the entire contents of the file --- @param lines string[] function Obj:stage_lines(lines) - local stdout = self:command({ + local stdout = self.repo:command({ 'hash-object', '-w', '--path', @@ -622,7 +595,7 @@ function Obj:stage_lines(lines) local new_object = stdout[1] - self:command({ + self.repo:command({ 'update-index', '--cacheinfo', string.format('%s,%s,%s', self.mode_bits, new_object, self.relpath), @@ -636,9 +609,7 @@ end function Obj:stage_hunks(hunks, invert) ensure_file_in_index(self) - local gs_hunks = require('gitsigns.hunks') - - local patch = gs_hunks.create_patch(self.relpath, hunks, self.mode_bits, invert) + local patch = require('gitsigns.hunks').create_patch(self.relpath, hunks, self.mode_bits, invert) if not self.i_crlf and self.w_crlf then -- Remove cr @@ -647,7 +618,7 @@ function Obj:stage_hunks(hunks, invert) end end - self:command({ + self.repo:command({ 'apply', '--whitespace=nowarn', '--cached', @@ -662,7 +633,7 @@ end --- @return string? function Obj:has_moved() - local out = self:command({ 'diff', '--name-status', '-C', '--cached' }) + local out = self.repo:command({ 'diff', '--name-status', '-C', '--cached' }) local orig_relpath = self.orig_relpath or self.relpath for _, l in ipairs(out) do local parts = vim.split(l, '%s+') diff --git a/lua/gitsigns/git/blame.lua b/lua/gitsigns/git/blame.lua index e32cd43ec..5a28e304e 100644 --- a/lua/gitsigns/git/blame.lua +++ b/lua/gitsigns/git/blame.lua @@ -199,7 +199,7 @@ function M.run_blame(obj, lines, lnum, revision, opts) process_incremental(data, commits, ret) end - local _, stderr = obj:command(args, { stdin = lines, stdout = on_stdout, ignore_error = true }) + local _, stderr = obj.repo:command(args, { stdin = lines, stdout = on_stdout, ignore_error = true }) if stderr then error_once('Error running git-blame: ' .. stderr) diff --git a/test/actions_spec.lua b/test/actions_spec.lua index c2f716179..4fa05299e 100644 --- a/test/actions_spec.lua +++ b/test/actions_spec.lua @@ -51,7 +51,7 @@ local function expect_hunks(exp_hunks) end describe('actions', function() - local config + local config --- @type Gitsigns.Config before_each(function() clear() @@ -308,10 +308,8 @@ describe('actions', function() local newfile = helpers.newfile exec_lua([[vim.g.editorconfig = false]]) system("printf 'This is a file with no nl at eof' > " .. newfile) - helpers.gitm({ - { 'add', newfile }, - { 'commit', '-m', 'commit on main' }, - }) + helpers.git({ 'add', newfile }) + helpers.git({ 'commit', '-m', 'commit on main' }) edit(newfile) check({ status = { head = 'master', added = 0, changed = 0, removed = 0 } }) diff --git a/test/gitdir_watcher_spec.lua b/test/gitdir_watcher_spec.lua index 9c97813cf..c03c7b3ee 100644 --- a/test/gitdir_watcher_spec.lua +++ b/test/gitdir_watcher_spec.lua @@ -147,8 +147,8 @@ describe('gitdir_watcher', function() helpers.write_to_file(f1, { '1', '2', '3' }) helpers.write_to_file(f2, { '1', '2', '3' }) - helpers.gitf({ 'add', f1, f2 }) - helpers.gitf({ 'commit', '-m', 'init commit' }) + git({ 'add', f1, f2 }) + git({ 'commit', '-m', 'init commit' }) setup_gitsigns(test_config) @@ -165,7 +165,7 @@ describe('gitdir_watcher', function() helpers.check({ signs = { changed = 1 } }, b1) helpers.check({ signs = { changed = 1 } }, b2) - helpers.gitf({ 'add', f1, f2 }) + git({ 'add', f1, f2 }) helpers.check({ signs = {} }, b1) helpers.check({ signs = {} }, b2) diff --git a/test/gitsigns_spec.lua b/test/gitsigns_spec.lua index 108f5faab..370de6539 100644 --- a/test/gitsigns_spec.lua +++ b/test/gitsigns_spec.lua @@ -18,7 +18,6 @@ local edit = helpers.edit local cleanup = helpers.cleanup local test_file = helpers.test_file local git = helpers.git -local gitm = helpers.gitm local scratch = helpers.scratch local newfile = helpers.newfile local match_dag = helpers.match_dag @@ -84,6 +83,7 @@ describe('gitsigns (with screen)', function() end) it('gitdir watcher works on a fresh repo', function() + --- @type integer local nvim_ver = exec_lua('return vim.version().minor') screen:try_resize(20, 6) setup_test_repo({ no_add = true }) @@ -157,7 +157,7 @@ describe('gitsigns (with screen)', function() it('can setup mappings', function() edit(test_file) expectf(function() - local res = split(helpers.api.nvim_exec('nmap ', true), '\n') + local res = split(helpers.api.nvim_exec2('nmap ', { output = true }).output, '\n') table.sort(res) -- Check all keymaps get set @@ -279,10 +279,8 @@ describe('gitsigns (with screen)', function() system("printf 'This\nis\na\nwindows\nfile\n' > " .. newfile) end - gitm({ - { 'add', newfile }, - { 'commit', '-m', 'commit on main' }, - }) + git({ 'add', newfile }) + git({ 'commit', '-m', 'commit on main' }) edit(newfile) feed('gg') @@ -363,10 +361,8 @@ describe('gitsigns (with screen)', function() feed('oEDIT') command('write') - gitm({ - { 'add', test_file }, - { 'commit', '-m', 'commit on main' }, - }) + git({ 'add', test_file }) + git({ 'commit', '-m', 'commit on main' }) -- Don't setup gitsigns until the repo has two commits setup_gitsigns(config) @@ -578,25 +574,24 @@ describe('gitsigns (with screen)', function() check({ status = { head = 'master', added = 0, changed = 1, removed = 0 } }) command('write') command('bwipe') - gitm({ - { 'add', test_file }, - { 'commit', '-m', 'commit on main' }, - -- Create a branch, remove last commit, edit file again - { 'checkout', '-B', 'abranch' }, - { 'reset', '--hard', 'HEAD~1' }, - }) + git({ 'add', test_file }) + git({ 'commit', '-m', 'commit on main' }) + + -- Create a branch, remove last commit, edit file again + git({ 'checkout', '-B', 'abranch' }) + git({ 'reset', '--hard', 'HEAD~1' }) + edit(test_file) check({ status = { head = 'abranch', added = 0, changed = 0, removed = 0 } }) feed('idiff') check({ status = { head = 'abranch', added = 0, changed = 1, removed = 0 } }) command('write') command('bwipe') - gitm({ - { 'add', test_file }, - { 'commit', '-m', 'commit on branch' }, - { 'rebase', 'master' }, - }) + + git({ 'add', test_file }) + git({ 'commit', '-m', 'commit on branch' }) + git({ 'rebase', 'master' }) -- test_file should have a conflict edit(test_file) @@ -711,10 +706,8 @@ describe('gitsigns (with screen)', function() write_to_file(uni_filename, { 'Lorem ipsum' }) - gitm({ - { 'add', uni_filename }, - { 'commit', '-m', 'another commit' }, - }) + git({ 'add', uni_filename }) + git({ 'commit', '-m', 'another commit' }) edit(uni_filename) diff --git a/test/gs_helpers.lua b/test/gs_helpers.lua index f8a6ecb54..d82b2819f 100644 --- a/test/gs_helpers.lua +++ b/test/gs_helpers.lua @@ -60,22 +60,14 @@ local test_file_text = { } --- Run a git command -function M.gitf(args) +local function git(args) system({ 'git', '-C', M.scratch, unpack(args) }) end ---- @param cmds string[][] -function M.gitm(cmds) - for _, cmd in ipairs(cmds) do - M.gitf(cmd) - end - helpers.sleep(10) -end - --- Run a git command and add a delay function M.git(args) - M.gitf(args) - helpers.sleep(10) + git(args) + -- helpers.sleep(10) end function M.cleanup() @@ -83,26 +75,26 @@ function M.cleanup() end function M.git_init() - M.gitf({ 'init', '-b', 'master' }) + git({ 'init', '-b', 'master' }) -- Always force color to test settings don't interfere with gitsigns systems -- commands (addresses #23) - M.gitf({ 'config', 'color.branch', 'always' }) - M.gitf({ 'config', 'color.ui', 'always' }) - M.gitf({ 'config', 'color.diff', 'always' }) - M.gitf({ 'config', 'color.interactive', 'always' }) - M.gitf({ 'config', 'color.status', 'always' }) - M.gitf({ 'config', 'color.grep', 'always' }) - M.gitf({ 'config', 'color.pager', 'true' }) - M.gitf({ 'config', 'color.decorate', 'always' }) - M.gitf({ 'config', 'color.showbranch', 'always' }) - - M.gitf({ 'config', 'merge.conflictStyle', 'merge' }) - - M.gitf({ 'config', 'user.email', 'tester@com.com' }) - M.gitf({ 'config', 'user.name', 'tester' }) - - M.gitf({ 'config', 'init.defaultBranch', 'master' }) + git({ 'config', 'color.branch', 'always' }) + git({ 'config', 'color.ui', 'always' }) + git({ 'config', 'color.diff', 'always' }) + git({ 'config', 'color.interactive', 'always' }) + git({ 'config', 'color.status', 'always' }) + git({ 'config', 'color.grep', 'always' }) + git({ 'config', 'color.pager', 'true' }) + git({ 'config', 'color.decorate', 'always' }) + git({ 'config', 'color.showbranch', 'always' }) + + git({ 'config', 'merge.conflictStyle', 'merge' }) + + git({ 'config', 'user.email', 'tester@com.com' }) + git({ 'config', 'user.name', 'tester' }) + + git({ 'config', 'init.defaultBranch', 'master' }) end --- Setup a basic git repository in directory `helpers.scratch` with a single file @@ -116,10 +108,10 @@ function M.setup_test_repo(opts) system({ 'touch', M.test_file }) M.write_to_file(M.test_file, text) if not (opts and opts.no_add) then - M.gitf({ 'add', M.test_file }) - M.gitf({ 'commit', '-m', 'init commit' }) + git({ 'add', M.test_file }) + git({ 'commit', '-m', 'init commit' }) end - helpers.sleep(20) + -- helpers.sleep(20) end --- @param cond fun() diff --git a/test/highlights_spec.lua b/test/highlights_spec.lua index b980b5133..769e18942 100644 --- a/test/highlights_spec.lua +++ b/test/highlights_spec.lua @@ -16,7 +16,7 @@ helpers.env() describe('highlights', function() local screen --- @type test.screen - local config + local config --- @type Gitsigns.Config before_each(function() clear() @@ -84,15 +84,6 @@ describe('highlights', function() p('Deriving GitSignsDeleteNr from GitSignsDelete'), }) end) - - -- eq('GitSignsChange xxx links to DiffChange', - -- exec_capture('hi GitSignsChange')) - - -- eq('GitSignsDelete xxx links to DiffDelete', - -- exec_capture('hi GitSignsDelete')) - - -- eq('GitSignsAdd xxx links to DiffAdd', - -- exec_capture('hi GitSignsAdd')) end) it('update when colorscheme changes', function() @@ -106,35 +97,5 @@ describe('highlights', function() config.linehl = true setup_gitsigns(config) - - -- expectf(function() - -- eq('GitSignsChange xxx links to DiffChange', - -- exec_capture('hi GitSignsChange')) - - -- eq('GitSignsDelete xxx links to DiffDelete', - -- exec_capture('hi GitSignsDelete')) - - -- eq('GitSignsAdd xxx links to DiffAdd', - -- exec_capture('hi GitSignsAdd')) - - -- eq('GitSignsAddLn xxx links to DiffAdd', - -- exec_capture('hi GitSignsAddLn')) - -- end) - - -- command('colorscheme blue') - - -- expectf(function() - -- eq('GitSignsChange xxx links to DiffChange', - -- exec_capture('hi GitSignsChange')) - - -- eq('GitSignsDelete xxx links to DiffDelete', - -- exec_capture('hi GitSignsDelete')) - - -- eq('GitSignsAdd xxx links to DiffAdd', - -- exec_capture('hi GitSignsAdd')) - - -- eq('GitSignsAddLn xxx links to DiffAdd', - -- exec_capture('hi GitSignsAddLn')) - -- end) end) end) diff --git a/test/unit_spec.lua b/test/unit_spec.lua deleted file mode 100644 index f74eb550f..000000000 --- a/test/unit_spec.lua +++ /dev/null @@ -1,71 +0,0 @@ -local helpers = require('test.gs_helpers') -local exec_lua = helpers.exec_lua -local setup_gitsigns = helpers.setup_gitsigns -local clear = helpers.clear - -helpers.env() - -local function get_tests(pattern) - local modules = exec_lua([[return vim.tbl_keys(package.loaded)]]) - - local tests = {} - for _, mod in ipairs(modules) do - if mod:match(pattern) then - tests[mod] = exec_lua( - [[ - local mod = package.loaded[...] - if type(mod) == 'table' then - return vim.tbl_keys(mod._tests or {}) - end - return {} - ]], - mod - ) - end - end - return tests -end - -local function run_test(mod, test) - return unpack(exec_lua( - [[ - local mod, test = ... - return {pcall(package.loaded[mod]._tests[test])} - ]], - mod, - test - )) -end - -local function load(mod) - exec_lua([[require(...)]], mod) -end - -describe('unit test', function() - clear() - exec_lua('package.path = ...', package.path) - exec_lua('_TEST = true') - setup_gitsigns({ debug_mode = true }) - - -- Add modules which have unit tests - -- TODO(lewis6991): automate - load('gitsigns.test') - - local gs_tests = get_tests('^gitsigns') - - for mod, tests in pairs(gs_tests) do - for _, test in ipairs(tests) do - it(mod .. ':' .. test, function() - local ok, err = run_test(mod, test) - - if not ok then - local msgs = helpers.debug_messages() - for _, msg in ipairs(msgs) do - print(msg) - end - error(err) - end - end) - end - end -end)