Skip to content
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.

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

Unified diff format is not supported for external diffs #14521

Open
gregorias opened this issue May 9, 2021 · 3 comments
Open

Unified diff format is not supported for external diffs #14521

gregorias opened this issue May 9, 2021 · 3 comments
Labels
bug issues reporting wrong behavior diff

Comments

@gregorias
Copy link
Sponsor Contributor

gregorias commented May 9, 2021

  • nvim --version: NVIM v0.5.0-dev+1302-g8a93d1028

Steps to reproduce using nvim -u NORC

" s.vim
function MyDiff()
  silent execute "!diff  -u " . v:fname_in . " " . v:fname_new . " > " . v:fname_out
endfunction
set diffexpr=MyDiff()
# Steps to reproduce
nvim -u NORC -O a b
# Inside Neovim:
:source s.vim
:windo diffthis

Actual behaviour

Vim outputs E97: Cannot create diffs

Expected behaviour

Vim turns on the diff mode without producing an error.

Notes

This is a bug, because documentation claims that Neovim should handle unified diffs (https://neovim.io/doc/user/diff.html, C^F "unified").

Neovim does handle unified diffs for internal diffs since cf1ffa9, but it seems that this functionality has not been plugged into external diffs, and there's even a smoke check that tests that the external diff complies with the ed-style format:

neovim/src/nvim/diff.c

Lines 985 to 992 in 1186f7d

// There must be a line that contains "1c1".
if (vim_fgets(linebuf, LBUFLEN, fd)) {
break;
}
if (STRNCMP(linebuf, "1c1", 3) == 0) {
ok = kTrue;
}

@gregorias gregorias added the bug issues reporting wrong behavior label May 9, 2021
glacambre added a commit to glacambre/neovim that referenced this issue May 10, 2021
Neovim is able to handle unified diffs fine - the only problem is that
it didn't check for unified diffs when making sure that the diff tool
worked.

Closes neovim#14521
@gregorias
Copy link
Sponsor Contributor Author

gregorias commented May 16, 2021

Thank you, @glacambre, for preparing a commit that solves this issue.

My intention is to go a step further and refactor the diff.c code a bit. In particular I intend to extract the diff parsing code from diff_read. Right now diff parsing has at least two following code smells:

  1. Diff parsing is entangled with updating buffer specific diff structures in diff_read.
  2. Parsing logic is in two places check_external_diff and diff_read.

A better structure would be to have one generic diff_parse that parses unified and and ed-style diffs into a generic diff structure, and check_external_diff and diff_read rely on diff_parse. It should make the code easier to read, and future-proof against adding another diff format.

@gregorias
Copy link
Sponsor Contributor Author

In #14614 I've extracted diff_parse so that it can be shared across check_external_diff and diff_read. This way external diffs will always be able to use the same formatting that internal diffs can.

@zeertzjq zeertzjq added the diff label Jan 17, 2022
@lewis6991
Copy link
Member

vim-patch 8.2.2880 was merged in 6d932cc

Does that fix this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues reporting wrong behavior diff
Projects
None yet
Development

No branches or pull requests

3 participants