ntpeters / vim-better-whitespace

Better whitespace highlighting for Vim
The Unlicense
1.37k stars 82 forks source link

unable to save file after recent updates #164

Open eirnym opened 9 months ago

eirnym commented 9 months ago

Today I've udpated the plugin and since then I'm unable to use plugin in Vim to strip whitespaces on save.

my environment:

When EnableStripWhiteSpaceOnSave is enabled I see following repeating lines:

Error detected while processing BufWrite Autocommands for "*"..function <SNR>33_StripWhitespaceOnSave:
line    8:
E684: List index out of range: 1
E116: Invalid arguments for function <SID>DetectWhitespace(r[0], r[1])
E684: List index out of range: 1
E116: Invalid arguments for function <SID>DetectWhitespace(r[0], r[1])
E684: List index out of range: 1

To narrow down the problem I checked the following:

  1. Removed any automatic remove whitespace on save configuration via an external editor.
  2. added a space at the end of a random line in a random file
  3. Checked commands: StripWhitespace and StripWhitespaceOnChangedLines
  4. StripWhitespace command works well
  5. While StripWhitespaceOnChangedLines produces following repeating output:
Error detected while processing function <SNR>33_StripWhitespaceOnChangedLinesCommand:
line    8:
E684: List index out of range: 1
line   11:
E684: List index out of range: 1
E116: Invalid arguments for function min([a:line2, r[1]]))
E116: Invalid arguments for function <SNR>33_StripWhitespace
line    8:
E684: List index out of range: 1
line   11:
E684: List index out of range: 1
E116: Invalid arguments for function min([a:line2, r[1]]))
E116: Invalid arguments for function <SNR>33_StripWhitespace
line    8:                                               
eirnym commented 9 months ago

While investigating further, I've echoed range element (local r variable) and found that diff options are not recognized. Somehow these errors were silenced and it was ok for me before.

However, looking on how to avoid the problem, I found that Vim has diff-original-file command which can be used to obtain changed lines in a cross platform way. I don't know if this command is supported by NeovVim as I don't use it. If it's not supported, I prefer to have a Vim command for Vim as it's my main tool to edit files.

Could you please update your script to that cross-platform method?

Cimbali commented 9 months ago

The option you link to would work if you’d like to have a visual diff window open and highlighting of changed lines. (Even if we’re closing the diff afterwards it certainly would cause massive visual disruption.) Also since we want to compare the current (modified) buffer to its corresponding file on disk I’m not sure how we’d open the file without it being a copy of the buffer (i.e. with the same modifications applied).

In any case, vim is calling diff to setup these visual diffs, so using diff isn’t the issue itself. The issue is that we’re using options to get exactly the info we want, and that apparently have never been added to the mac version.

The fix would be to have a different code path on macOS to determine the modified lines that:

There might be other options on mac’s diff that make this parsing easier (maybe -e to generate an ed script? or -n for RCS format?) but I don’t have a mac to test these on. And it’s hard to find out what’s what without a mac, for example this online man page of mac diff specifically lists all the options we want (--new-group-format etc.) as if they were available.

Until the moment someone takes the time to write up an appropriate PR for this, the main workaround remains to install gnu diff on mac, as mentioned in other issues on this topic. It doesn’t have to be in your path and disrupt your workflow, you can dump it in any folder and just point g:diff_binary to it. Maybe there even is a fully featured diff distributed as part of XCode, I don’t know – in which case you can just set g:diff_binary.

Cimbali commented 9 months ago

Actually @eirnym can you copy here the outputs of diff --version and diff --help? That may be a good reliable starting point.

eirnym commented 9 months ago
$ diff --version
Apple diff (based on FreeBSD diff)
$ diff --help
usage: diff [-aBbdilpTtw] [-c | -e | -f | -n | -q | -u] [--ignore-case]
            [--no-ignore-case] [--normal] [--strip-trailing-cr] [--tabsize]
            [-I pattern] [-F pattern] [-L label] file1 file2
       diff [-aBbdilpTtw] [-I pattern] [-L label] [--ignore-case]
            [--no-ignore-case] [--normal] [--strip-trailing-cr] [--tabsize]
            [-F pattern] -C number file1 file2
       diff [-aBbdiltw] [-I pattern] [--ignore-case] [--no-ignore-case]
            [--normal] [--strip-trailing-cr] [--tabsize] -D string file1 file2
       diff [-aBbdilpTtw] [-I pattern] [-L label] [--ignore-case]
            [--no-ignore-case] [--normal] [--tabsize] [--strip-trailing-cr]
            [-F pattern] -U number file1 file2
       diff [-aBbdilNPprsTtw] [-c | -e | -f | -n | -q | -u] [--ignore-case]
            [--no-ignore-case] [--normal] [--tabsize] [-I pattern] [-L label]
            [-F pattern] [-S name] [-X file] [-x pattern] dir1 dir2
       diff [-aBbditwW] [--expand-tabs] [--ignore-all-blanks]
            [--ignore-blank-lines] [--ignore-case] [--minimal]
            [--no-ignore-file-name-case] [--strip-trailing-cr]
            [--suppress-common-lines] [--tabsize] [--text] [--width]
            -y | --side-by-side file1 file2
       diff [--help] [--version]
eirnym commented 9 months ago

@Cimbali yes, Vim is calling diff underneath, but support multiple versions without any problem. The diff options supplied with the plugin most likely applicable to the gnu diff after some specific version. My guess the diff options won't work on FreeBSD as well.

Cimbali commented 9 months ago

I’d say give this a go (not fully tested):

1) Remove group options

" Diff command returning a space-separated list of ranges of new/modified lines (as first,last)
let s:diff_cmd=g:diff_binary.' -a'

2) Do the parsing in vim

" Get the ranges of changed lines
function! s:ChangedLines()
    if !filereadable(expand('%'))
        return [[1,line('$')]]
    elseif &modified
        redir => l:better_whitespace_changes_list
            silent! echo system(s:diff_cmd.' '.shellescape(expand('%')).' -', join(getline(1, line('$')), "\n") . "\n")
        redir END
        let lines=l:better_whitespace_changes_list->split('\n')->filter('match(v:val, "^[0-9,]*[ac][0-9,]*$") > -1')
        return lines->map('split(split(v:val, "[ac]")[1], ",")')->map('len(v:val) == 1 ? v:val + v:val : v:val')
    endif
    return []
endfunction
srgsanky commented 7 months ago

I followed https://github.com/NixOS/nix/issues/7286 to install diffutils https://formulae.brew.sh/formula/diffutils

Before

diff --version
Apple diff (based on FreeBSD diff)

After installing diffutils using brew install diffutils

diff --version
diff (GNU diffutils) 3.10

Now I am able to strip whitespace on save without any errors.

The error message could have been better to show that the diff option is not support.