SVG status about gitattributes HOT 20 CLOSED

Obvious for me. SVG is text

from gitattributes.

@compumike08 https://github.com/compumike08 I see your reasoning, but a
similar argument could be made for Markdown files, right? Yet we consider
those text files.

from gitattributes.

@JacksonBailey

You make some valid points, however there is one other difference between how binary and text files are handled besides how they appear during diff. Binary and text types vary in how line ending conversions are handled. Depending on the project's settings for handling end of line conversions, text files may have their line endings altered when committed and/or checked out of a repository. Binary files do not have the line endings touched. Generally speaking, even though SVG files are technically XML, they don't normally need to have their line endings converted based on the type of system they are being checked out on because line ending types only matter if a file is going to be edited after the initial commit. Also, if the line ending conversion settings for a repository aren't set properly, you can potentially have the situation where an SVG file is initially committed on a Windows machine, and then checked out on a Mac.
Then the Mac user decides to update the SVG file (they replace the file with a completely new one, but the file has the same name). Then they commit it. When the Windows user then pulls that commit, the SVG file may constantly show as if the Windows user has uncommitted changes in their working directory for the SVG file even though they haven't touched it. Now, it is true that if the proper configuration for line ending conversion for the type of machine each user is working on is set, this problem will not happen. However, many people don't have those settings set properly. We had this issue happen to us at my work, and it caused a lot of annoying problems that were difficult to track down. Now, of course this can happen to other file types as well if the correct line ending settings aren't in place, but if SVG files are left as text type then it is just adding another place for trouble to happen. On my team at work, all of the software developers use Windows, but the UI/UX developers use Macs. They do a lot of commits to our repositories with updating and changing image files, and they aren't as fluent in Git's details as the software developers (they know how to use the basics of Git, but they don't have an in-depth understanding of all the idiosyncrasies and complications of Git). They ended up messing up our repositories because they kept overriding Git's warnings about the "uncommitted changes in the working directory" regarding SVG files because they knew they hadn't changed them and thought that by forcibly overriding Git pull that they were wiping the uncommitted changes (because that is what Git warns will happen if you forcibly override that warning message). Since at
the time Git was treating SVG files as
text, it was applying the line ending transformations to them on the Windows machines. Because the Mac people didn't have their core.autocrlf set to "input", Git STILL thought those files were changed (even though they weren't), and when they did "git add ." later to stage and commit their changes, they didn't realize the SVG files were marked as staged and committed as well (they changed a lot of files at once, so it was easy to overlook them in the list of staged changes). This ended up committing the line ending changes into our repository and screwing up the files on the Windows machines. It took as a long time to track down the cause of this problem, because no one initially made the connection between SVG files and line ending conversions because most people think of SVG files as any other image file unless they stop and think about it carefully. It took as weeks of investigating this problem before I finally made the connection that since SVG files are technically XML, Git might be autodetecting them as text and applying the line ending conversions. We ended up having to heavily edit our Git history to fix the problem. Since then we tell all our new developers to properly set the line endings for their system, but we also add all image files (including SVG) as binary explicitly to prevent this from happening again in case someone forgets to properly configure their line ending settings.

In the handful of cases where people DO need to edit their SVG files and DO need line ending conversion applied to those files, they can set SVG as text in their repositories. But the majority of people do not need to do that, so why introduce a potential pain point than can be avoided.

from gitattributes.

What about the option (semi binary): *.svg -text diff -merge

On windows machine checkout didn''t change lf:

MINGW64 /c/dev/projects/github/qualinsight-plugins-sonarqube-badges (svg-test-binary-allow-diff)
$ git ls-files --eol | grep images/measure_rounded_neutral.svg
i/lf    w/lf    attr/-text              images/measure_rounded_neutral.svg

But diff is possible:

$ git show
commit ca79d957d940d2546152d5fc2c4a880467b40042 (HEAD -> svg-test-binary-allow-diff, origin/svg-test-binary-allow-diff, svg-test)

    Revert color change.

diff --git a/images/measure_rounded_neutral.svg b/images/measure_rounded_neutral.svg
index ed5aad9..2ff92e2 100644
--- a/images/measure_rounded_neutral.svg
+++ b/images/measure_rounded_neutral.svg
@@ -10,7 +10,7 @@
   </mask>
   <g mask="url(#round)">
     <rect fill="#444" height="20" width="81" />
-    <rect fill="#4b9fd5" height="20" width="32" x="81" />
+    <rect fill="#999" height="20" width="32" x="81" />
     <rect fill="url(#smooth)" height="20" width="113" />
   </g>
   <g fill="#fff" font-family="DejaVu Sans,Verdana,Sans PT,Lucida Grande,Tahoma,Helvetica,Arial,sans-serif" font-size="11" text-anchor="middle">

from gitattributes.

I had guessed the reasoning in favor of it being binary is along the lines of:

• Graphics/Images are assets, so treat them as "binary".
• Every other image is "binary".

But, let's ask :).
@compumike08
@pazkooda

Thoughts?

from gitattributes.

SVG format is in XML. It should definitely be text.

SVG is a language for describing two-dimensional graphics in XML [XML10].

Reference

Here is an example from Wikipedia. You can save it and open it in a text editor and see that it's definitely not binary. Below is the image and source.

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="391" height="391" viewBox="-70.5 -70.5 391 391" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<defs>
    <pattern id="grid" patternUnits="userSpaceOnUse" width="50" height="50">
        <rect x="0" y="0" width="50" height="1" fill="#000" opacity="1.0"/>
        <rect x="0" y="0" width="1" height="50" fill="#000" opacity="1.0"/>
    </pattern>
    <pattern id="dots" patternUnits="userSpaceOnUse" width="50" height="50">
        <g fill-opacity="0.40" stroke="#000" stroke-dasharray="0.3,0.3">
            <circle cx="0" cy="0" r="4" fill="red"/>
            <circle cx="51" cy="0" r="4" fill="blue"/>
            <circle cx="51" cy="51" r="4" fill="green"/>
            <circle cx="0" cy="51" r="4" fill="yellow"/>
        </g>
    </pattern>
</defs>
<rect fill="#fff" stroke="#000" x="-70" y="-70" width="390" height="390"/>
<rect fill="url(#dots)" x="0" y="0" width="250" height="250"/>
<rect fill="url(#grid)" stroke-width="2" stroke="#000" x="0" y="0" width="250" height="250"/>
<text x="0" y="0" text-anchor="middle" font-size="16"  font-family="Times New Roman,serif">
<tspan x="125" y="-40" font-weight="bold">X</tspan>
<tspan x="0" y="-10">0</tspan>
<tspan x="50" y="-10">50</tspan>
<tspan x="100" y="-10">100</tspan>
<tspan x="150" y="-10">150</tspan>
<tspan x="200" y="-10">200</tspan>
<tspan x="250" y="-10">250</tspan>
</text>
<text x="0" y="0" text-anchor="middle" font-size="16" font-family="Times New Roman,serif">
<tspan x="-50" y="125" font-weight="bold">Y</tspan>
<tspan x="-20" y="0" dy="6">0</tspan>
<tspan x="-20" y="50" dy="6">50</tspan>
<tspan x="-20" y="100" dy="6">100</tspan>
<tspan x="-20" y="150" dy="6">150</tspan>
<tspan x="-20" y="200" dy="6">200</tspan>
<tspan x="-20" y="250" dy="6">250</tspan>
</text>
<g opacity="0.8">
    <rect x="25" y="25" width="200" height="200" fill="lime" stroke-width="4" stroke="pink" />
    <circle cx="125" cy="125" r="75" fill="orange" />
    <polyline points="50,150 50,200 200,200 200,100" stroke="red" stroke-width="4" fill="none" />
    <line x1="50" y1="50" x2="200" y2="200" stroke="blue" stroke-width="4" />
</g>
</svg>

from gitattributes.

I realize that SVG is technically an XML file format. My argument for SVG being treated as binary is based on the Git manual. I've copied and pasted the explanation I included in the pull request in which I originally changed SVG to binary type:

Although SVG files are technically based on XML files (and so Git is technically capable of treating them as text files instead of binary), the best way for Git to handle SVG files is as binary. The Git manual page for gitattributes says in the section "Marking files as binary" that some types of files, while technically text files, can be marked as binary when their content is "opaque" to human readers (I'm paraphrasing the actual text from the Git manual). SVG files aren't meant to be viewed in their raw form by human users, and you will very rarely ever need to perform a diff on an SVG file. While I'm sure there are rare edge cases out there where someone has an unusual use-case that would benefit from treating SVG files as text in Git, the vast majority of users won't ever need to and treating them as text can just produce noisy and useless diffs. As this GitHub repository is intended to be a library of template .gitattributes files for others to use, I think these templates should reflect the most common use-cases. Those rare people who need to actually work with SVG files as text-based in Git can simply change it back to 'text' type in their local .gitattributes file that they are using in their actual project.

@danimoth
@Noctiflore
@JacksonBailey
@pazkooda

from gitattributes.

@compumike08 I see your point too ; I know some people who edit SVG files at hand, but I agree that most people just use them as "assets".
I don't know if we should revert to text or let the binary status, but in both cases we should put these ambiguous cases in a (comment-)separated section with a warning somewhere.

@danimoth
@Noctiflore
@JacksonBailey
@pazkooda

from gitattributes.

@JacksonBailey There is a difference between Markdown and SVG files. With Markdown, it is not unreasonable that people may need to do a diff on them; they may want to edit the file to add or remove things and then run git diff to see what changes to the Markdown files are going to be committed. With SVG, that will almost never happen (yes, there are the rare edge cases, but those would be so rare that I'd argue to be outside the scope of this general repository of .gitattirbutes templates).

@Noctiflore I'm okay with adding a warning or other comment to explain this to users, but I definitely think that the binary status should stand for this repository.

@danimoth
@pazkooda

from gitattributes.

How about explicitly calling out the ambiguity as shown in #50?

from gitattributes.

@compumike08, @danimoth : That's ok for me.

from gitattributes.

@danimoth @Noctiflore The comment added in #50 looks good to me too.

from gitattributes.

[...] With Markdown, it is not unreasonable that people may need to do a diff on them; they may want to [edit it] and then run git diff [...]. With SVG, that will almost never happen [...]

Noctiflore I'm okay with adding a warning or other comment to explain this to users, but I definitely think that the binary status should stand for this repository.

@compumike08 Your points are correct but, to me at least, your reasoning is wrong. Just because people don't hand-edit files doesn't mean they should be binary instead of text.

As far as I know the only benefit of making the file text or binary (apart from EOL correction) is how it displays its differences in the diff view. Binary files aren't shown by default in this view. My guesses why are:

1. Because they are not considered human readable so what the change is about wouldn't be discernible
2. Binary files tend to be the sort of files where a small change change drastically changes all the contents of the file (think compiled or compressed files)

On those notes, SVG is human readable. So what changed is discernible form the diff. In addition, SVG is not the type of file that has huge changes when a small "real" change occurs.

So, I'm still not convinced that binary makes the most sense, even in the use cases you've described (which I accept are likely the default use cases).

As an aside, I do agree that the most-likely default use case should drive what a template repository like this does, I just still think text makes more sense for SVG. So for some other hypothetical extension, say foo, that was some sort of compressed file format but rendered as text, I would agree that binary makes more sense even though the character set is text.

from gitattributes.

In the handful of cases where people DO need to edit their SVG files and DO need line ending conversion applied to those files, they can set SVG as text in their repositories. But the majority of people do not need to do that, so why introduce a potential pain point than can be avoided.

@compumike08 Your solution with binary is equivalent to -text -diff which permits committing the file without normalizing the line endings when stored in the repo(-text is equivalent to core.autocrlf=false).

So I'm not really seeing how that's a solution to avoid the issue you described. It was the -diff part that was avoiding any noticeable diffs appearing because of the actual problem of different line endings being committed. So it sounds like you've just hidden the problem you experienced.

Checkout wise, you'd get the SVG with CRLF if it was committed as that, or when committed as LF, you might get LF during checkout(core.autocrlf as input, or as false with core.eol as lf, or native(default) when on linux/mac, otherwise CRLF on Windows).

Checkin(git add/git commit), will not normalize, but as described above, other software aside, you can get conversion on checkout easily.

It'd be better to normalize to LF when doing a checkin, so text should be preferred. Problem above averted. -diff is still useful if the text diffs aren't likely to be meaningful to the project.

*.svg text -diff

from gitattributes.

@alexkaratarakis At the moment, Common.gitattributes says SVG should be binary but Web.gitattributes says it should be text. What should the default be?

from gitattributes.

@Richienb *.svg text -diff

from gitattributes.

From all the community input at the moment, it seems that something like *.svg text should be the default option. I'm going to normalize the files now.

from gitattributes.

@Richienb there seems to be a small mistake in Common.gitattributes and Web.gitattributes after d1514be with regards to *.svg as shown ⬇️

https://github.com/alexkaratarakis/gitattributes/blob/cc0756087755535c2f9bd8ed854c69f36e2a6e4a/Common.gitattributes#L41-L45

First the attributes for .svg-files is set to "text". This is followed by a comment saying that "if you want to treat it as text (which it now already is), use the following line instead", and the following line shows you how to set the attributes for .svg-files to "binary".

from gitattributes.

@ericcornelissen Thanks for the report! I'm fixing it now.

from gitattributes.

@ericcornelissen Fixed in 6764538.

from gitattributes.