OmniSharp / omnisharp-vim

Vim omnicompletion (intellisense) and more for C#
http://www.omnisharp.net
MIT License
1.7k stars 169 forks source link

`:OmniSharpCodeFormat` BOM removal bug? #781

Closed rene-descartes2021 closed 2 years ago

rene-descartes2021 commented 2 years ago

Hi, I'm getting back into my C# project again :-) The ad-hoc Vimspector config is wonderful!

I noticed a subtle problem with :OmniSharpCodeFormat. Running :OmniSharpCodeFormat appears to remove the Byte Order Mark (BOM)?

To reproduce the problem, run :OmniSharpCodeFormat on this cs file, then run git diff. Some cs files in that repository have that BOM prefix, many don't, but that one is an example of one with. That is the state of that repository with inconsistent BOMs on the cs files , so not an issue my git or Vim config AFAIK.

With OmniSharp-vim, this happened both ~7 months ago and also with the recent fileencoding commits. I am using up-to-date OmniSharp-vim master (d322a7003613724d2cafffeb206d5ad2b7a08a37), so not a regression.

Below is a series of diffs on the same file to help illuminate the subtle change to the file that :OmniSharpCodeFormat does.


What git diff looks like visually after the BOM removal, no obvious change yet it is reported that the first line changes:

diff --git a/Robust.Shared/ProfileOptSetup.cs b/Robust.Shared/ProfileOptSetup.cs
index 995dc2975..d42eeccf0 100644
--- a/Robust.Shared/ProfileOptSetup.cs
+++ b/Robust.Shared/ProfileOptSetup.cs
@@ -1,4 +1,4 @@
-using System.Runtime;
+using System.Runtime;
 using Robust.Shared.Configuration;
 using Robust.Shared.ContentPack;

With git diff and screen ctrl+a,esc copy, ctrl+a,] paste into Vim. <feff> is a single character in Vim, the cursor skips over it. When copied from Vim to here it doesn't show up in the browser so I manually typed it.

diff --git a/Robust.Shared/ProfileOptSetup.cs b/Robust.Shared/ProfileOptSetup.cs
index 995dc2975..d42eeccf0 100644
--- a/Robust.Shared/ProfileOptSetup.cs
+++ b/Robust.Shared/ProfileOptSetup.cs
@@ -1,4 +1,4 @@
-<feff>using System.Runtime;
+using System.Runtime;
 using Robust.Shared.Configuration;
 using Robust.Shared.ContentPack;

With git diff | cat -v:

diff --git a/Robust.Shared/ProfileOptSetup.cs b/Robust.Shared/ProfileOptSetup.cs
index 995dc2975..d42eeccf0 100644
--- a/Robust.Shared/ProfileOptSetup.cs
+++ b/Robust.Shared/ProfileOptSetup.cs
@@ -1,4 +1,4 @@
-M-oM-;M-?using System.Runtime;
+using System.Runtime;
 using Robust.Shared.Configuration;
 using Robust.Shared.ContentPack;

Here is a StackOverflow explanation on M-oM-;M-?. Interesting frustrated comments from developers on tool problems.

nickspoons commented 2 years ago

Hi @rene-descartes2021, thanks for the very detailed issue description, it is much appreciated.

I've just done some work with keeping the BOM when generating new files using code actions: #780

It sounds like a couple more changes are needed when code actions modify the current file. I'll do some tests and see what I can do.

nickspoons commented 2 years ago

OK I've been trying to repro this and I can't.

I have the linked file, which contains the BOM. Rather than using diffs, I've just been using xxd (which ships with vim) to inspect the first line:

$ xxd ProfileOptSetup.cs | head -1
00000000: efbb bf75 7369 6e67 2053 7973 7465 6d2e  ...using System.
          ^^^^ ^^                                  ^^^
          BOM

I can remove the BOM by opening vim, checking that it has detected the BOM with :set bomb?, then :set nobomb and :w and now it's gone:

$ xxd ProfileOptSetup.cs | head -1
00000000: 7573 696e 6720 5379 7374 656d 2e52 756e  using System.Run
          ^                                        ^
          No BOM

Now I undo that change and add the BOM back to the file again, and run :OmniSharpCodeFormat and :w and check the file with xxd, but the BOM is still there.

What is your Operating System, and what are the values of bomb, binary, encoding, fileencoding, fileencodings when you open vim in this file? Mine look like this in linux:

:set bomb? binary? encoding? fileencoding? fileencodings?
nobomb
nobinary
  encoding=utf-8
  fileencoding=utf-8
  fileencodings=ucs-bom,utf-8,default,latin1
rene-descartes2021 commented 2 years ago

tl;dr: This is a bug in the liuchengxu/vim-better-default plugin and not OmniSharp-vim

Oh how interesting.

$ vim Robust.Shared/ProfileOptSetup.cs
:set bomb? binary? encoding? fileencoding? fileencodings?
nobomb
nobinary
  encoding=utf-8
  fileencoding=utf-8
  fileencodings=utf-8,ucs-bom,gb18030,gbk,gb2312,cp936

With the fileencodings manually set to what you have, the BOM isn't removed with :OmniSharpCodeFormat. That fileencodings setting you have also appears to be the default on my system (a GNU/Linux Debian distribution), when launching solely with the system vimrc (no plugins or user vimrc):

$ /usr/bin/vim -u /etc/vim/vimrc Robust.Shared/ProfileOptSetup.cs
:set bomb? binary? encoding? fileencoding? fileencodings?
  bomb
nobinary
  encoding=utf-8
  fileencoding=utf-8
  fileencodings=ucs-bom,utf-8,default,latin1

Looks like the plugin I have which changes the fileencodings is liuchengxu/vim-better-default in plugin/default.vim. linuchengxu also authored vim-clap, vista.vim, vim-which-key, space-vim.

Looks like gb18030,gbk,gb2312,cp936 are all encodings for Chinese letters. It looks like issue has to do with the reordering of ucs-bom,utf-8 as when I changed the ordering for ucs-bom to be first: :OmniSharpCodeFormat no longer removes the BOM for ProfileOptSetup.cs:

:set fileencodings=ucs-bom,utf-8,gb18030,gbk,gb2312,cp936
:set fileencodings?
fileencodings=ucs-bom,utf-8,gb18030,gbk,gb2312,cp936

"Open ProfileOptSetup.cs, then:
:OmniSharpCodeFormat
:set bomb? binary? encoding? fileencoding? fileencodings?
nobomb
nobinary
  encoding=utf-8
  fileencoding=utf-8
  fileencodings=ucs-bom,utf-8,gb18030,gbk,gb2312,cp936

"ProfileOptSetup.cs is unchanged, still has BOM!

After some reading on help fileencodings:

The special value "ucs-bom" can be used to check for a Unicode BOM (Byte Order Mark) at the start of the file. It must not be preceded by "utf-8" or another Unicode encoding for this to work properly.

So, this is a bug with the vim-better-default plugin and not OmniSharp-vim. Apologies.