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

unable to save file after recent updates #164

Open
eirnym opened this issue Jan 22, 2024 · 7 comments
Open

unable to save file after recent updates #164

eirnym opened this issue Jan 22, 2024 · 7 comments

Comments

@eirnym
Copy link

eirnym commented Jan 22, 2024

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

my environment:

  • macOS Sonoma with the latest Xcode installed
  • MacVim r179 (Vim 9.1.0)

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
Copy link
Author

eirnym commented Jan 22, 2024

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
Copy link
Collaborator

Cimbali commented Jan 22, 2024

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:

  • calls diff -a on the same inputs as we currently do
  • parses the whole diff output to get the lines of insertions and changes (ignoring unchanged and removed lines). Something like:
    • Ignore lines starting with >, <, or -
    • For added / changed lines, matching \d*(,\d*)*[ac]\d*(,\d*)* keep the part after the a or c character
    • Ignore deleted lines, matching \d*(,\d*)*[d]\d*(,\d*)*

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
Copy link
Collaborator

Cimbali commented Jan 22, 2024

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

@eirnym
Copy link
Author

eirnym commented Jan 22, 2024

$ 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
Copy link
Author

eirnym commented Jan 22, 2024

@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
Copy link
Collaborator

Cimbali commented Jan 22, 2024

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'
  1. 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
Copy link

I followed NixOS/nix#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants