8000 fix(setqflist): don't strip trailing newline from working tree state by marcuscaisey · Pull Request #879 · lewis6991/gitsigns.nvim · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(setqflist): don't strip trailing newline from working tree state #879

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

Jump to bottom
Merged
merged 1 commit into from
Sep 17, 2023
Merged

fix(setqflist): don't strip trailing newline from working tree state #879

merged 1 commit into from
Sep 17, 2023

Conversation

marcuscaisey
Copy link
Contributor

Summary

The fix of #779 in #862 broke setqflist. An empty hunk is being added to the quickfix for each changed file and no hunks is being added for a newline appended to a file.

Problem

Repo:get_show_text is used to retrieve the lines of the index state and utils.file_lines is used to retrieve the lines of the working tree state. The trailing empty line is being retained by Repo:get_show_text (by providing { raw = true } to Repo:command)

gitsigns.nvim/lua/gitsigns/git.lua

Lines 404 to 405 in 907ae86

function Repo:get_show_text(object, encoding)
local stdout, stderr = self:command({ 'show', object }, { raw = true, suppress_stderr = true })
but not utils.file_lines

gitsigns.nvim/lua/gitsigns/util.lua

Lines 29 to 35 in 907ae86

function M.file_lines(file)
local text = {} --- @type string[]
for line in io.lines(file) do
text[#text + 1] = line
end
return text
end
(io.lines strips trailing newlines by default).

This PR fixes the problem by adding the option to retain the trailing empty line to utils.file_lines as well. I can see that this method is only used by buildqflist at the moment so we could just make this the default behaviour but:

  1. This doesn't seem like the behaviour which you'd expect.
  2. This wouldn't be in keeping with Repo:command which strips the trailing empty line by default.

Reproduction of bug and fix

  1. Setup a test repo and clone the current and fixed states of the plugin. foo has had a trailing new line appended as in Newline at end of file is not shown #779 and bar has had its first line modified.
mkdir setqflist-newline && \
  cd setqflist-newline && \
  git clone https://github.com/lewis6991/gitsigns.nvim --depth 1 gitsigns/before && \
  git clone https://github.com/marcuscaisey/gitsigns.nvim.git --depth 1 --branch setqflist-newline gitsigns/after && \
  git init && \
  echo -n "a" > foo && \
  echo -n "a\nb\nc\n" > bar && \
  git add foo bar && \
  git commit -m "add foo and bar" && \
  echo -n "a\n" > foo && \
  echo -n "d\nb\nc\n" > bar
git diff
image
  1. Current behaviour
nvim --clean \
  -c "lua vim.opt.runtimepath:append('gitsigns/before')" \
  -c "lua require('gitsigns').setup()" \
  -c "lua require('gitsigns').setqflist('all')"
image
  1. Fixed behaviour
nvim --clean \
  -c "lua vim.opt.runtimepath:append('gitsigns/after')" \
  -c "lua require('gitsigns').setup()" \
  -c "lua require('gitsigns').setqflist('all')"
image

@marcuscaisey
Copy link
Contributor Author

The nightly check failed on #878 as well.

@lewis6991 lewis6991 merged commit bce4576 into lewis6991:main Sep 17, 2023
@lewis6991
Copy link
Owner

LGTM. Thanks for the fix and the explanation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marcuscaisey @lewis6991
0