pombreda / vcscommand

Automatically exported from code.google.com/p/vcscommand
0 stars 0 forks source link

Support Byte Order Mark with VCSVimDiff #89

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Take file under SVN control saved as UTF-8 with a Byte Order Mark (:set bomb)
2. Tell Vim to recognize the BOM :set fileencondings=ucs-bom.
3. Open the file in Vim
4. Use :VCSDiffVim to show the diff

What is the expected output? What do you see instead?
The BOM sequence should not be displayed in the buffer opened by VCS.
See the screenshot attached showing the defect.

What version of the product are you using? On what operating system?
I am using vcscommand-1.99.46.zip under Vim 7.3.46 and Win7.

Original issue reported on code.google.com by delphin....@gmail.com on 30 Jan 2012 at 11:38

Attachments:

GoogleCodeExporter commented 9 years ago
Just to be clear, is the file in question already committed with a BOM, and 
then further edited?  Or was the edit to add the BOM part of the difference 
from the committed version?

I have tried to replicate this without success.

Original comment by bob.hies...@gmail.com on 1 Feb 2012 at 4:37

GoogleCodeExporter commented 9 years ago
This file was supposed to be save and checked-in to version control with the 
BOM already.
Then I am just editing the local version of this file and opening the diff from 
the previous committed version (base) using the command :VCSVimDiff.
Maybe this is again related to the Vim settings...

Original comment by delphin....@gmail.com on 1 Feb 2012 at 4:49

GoogleCodeExporter commented 9 years ago
I see it now using SVN.  I was lazy and used an existing git repo.  I'll see 
what SVN is doing differently.

Original comment by bob.hies...@gmail.com on 1 Feb 2012 at 4:54

GoogleCodeExporter commented 9 years ago
Could you please try the attached replacement for vcscommand.vim?  It simply 
attempts to blindly remove any BOM appearing at the beginning of content 
retrieved from the VCS.

Original comment by bob.hies...@gmail.com on 1 Feb 2012 at 7:00

Attachments:

GoogleCodeExporter commented 9 years ago
It seems to work with your file attached.
Thanks

Original comment by delphin....@gmail.com on 2 Feb 2012 at 9:59

GoogleCodeExporter commented 9 years ago
Are there any other commands that cause similar issues?  For instance, I'd 
guess that annotate does.

Original comment by bob.hies...@gmail.com on 13 Feb 2012 at 4:30

GoogleCodeExporter commented 9 years ago
Annotate is causing the same issue (same command as Blame I guess?).

Original comment by delphin....@gmail.com on 13 Feb 2012 at 4:36

GoogleCodeExporter commented 9 years ago
That may make the change a little harder.  Removing the BOM from the file 
itself (when displayed in VCSVimDiff, for instance) is clearly correct.  
Removing it when it is not the first couple of characters of VCS output is not 
necessarily correct, though I doubt it would normally cause problems.

I'm tempted to remove BOMs from the start of the output by default, but only 
remove it if it's mixed in if a certain option is set in the vimrc.  Does that 
approach make sense?

Original comment by bob.hies...@gmail.com on 13 Feb 2012 at 7:24

GoogleCodeExporter commented 9 years ago
A version similar to what I described in (8) is available in the vcscommand.vim 
at:

http://repo.or.cz/w/vcscommand.git/blob_plain/refs/heads/bh/noBOM:/plugin/vcscom
mand.vim

Unless you set VCSCommandRemoveBOM to 0, it will remove a leading BOM.  If you 
set VCSCommandRemoveAllBOM to non-zero, it will remove any BOM it finds in VCS 
output.

Does this version work for you?

Original comment by bob.hies...@gmail.com on 13 Feb 2012 at 7:35

GoogleCodeExporter commented 9 years ago
I will try this tomorrow but actually I don't understand clearly what you mean 
by "Removing it when it is not the first couple of characters of VCS output is 
not necessarily correct"

Original comment by delphin....@gmail.com on 13 Feb 2012 at 7:59

GoogleCodeExporter commented 9 years ago
The VCSReview and VCSVimDiff commands work by pulling the entire file, without 
any additional markup, from the underlying version control system.  In that 
case, the BOM will be the first bytes in the file, and are easily recognized 
and removed.

In cases like VCSAnnotate or VCSSDiff, the contents of the file will appear 
with other markup (such as line numbers, etc), so the BOM will no longer be at 
the beginning of the file.  In these cases, I'm not certain that blindly 
removing the BOM is the right (and expected) action.

Original comment by bob.hies...@gmail.com on 13 Feb 2012 at 10:02

GoogleCodeExporter commented 9 years ago
I don't know what are the limitations with your implementation but you 
shouldn't remove the BOM blindly.
This should be done according to the filencodings setting.
If it starts with ucs-bom, then you should test the first bytes of the file and 
remove the bytes according to the different possible BOMs: 
http://en.wikipedia.org/wiki/Byte_order_mark. You may also automatically set 
the fileencoding of Vim buffer according to the BOM that you have found.

Original comment by delphin....@gmail.com on 13 Feb 2012 at 10:14

GoogleCodeExporter commented 9 years ago
There is no file encoding.  I have no way of knowing what each VCS is 
returning.  The list of 'fileencodings' is a list of possibilities.   What I am 
telling you is for VCS calls like 'diff' or 'blame', the BOM will not be at the 
beginning of the output.

Original comment by bob.hies...@gmail.com on 13 Feb 2012 at 10:33

GoogleCodeExporter commented 9 years ago
Indeed, the fileencodings is a list of possibilities.
My suggestion was to provide a smart mode when the fileencodings list starts 
with ucs-bom.
Then, you may use a regular expression or something like that in order to match 
the beginning of the first line excluding the stuff added by diff or blame.
Anyway, this is just a suggestion but we can live with this.

Original comment by delphin....@gmail.com on 13 Feb 2012 at 11:06

GoogleCodeExporter commented 9 years ago
VCSCommandRemoveAllBOM worked.

Original comment by delphin....@gmail.com on 15 Feb 2012 at 5:53

GoogleCodeExporter commented 9 years ago

Original comment by bob.hies...@gmail.com on 12 Apr 2013 at 3:00