Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Ensure Docker script files uses LF to support Docker for Windows.
setup_docker_prereqs eol=lf
/virtualization/Docker/scripts/* eol=lf
# Ensure "git config --global core.autocrlf input" before you clone
setup_docker_prereqs text eol=lf

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we just force this for all files instead of pick and choose?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is indeedwhat github recommends, but then you should EXCLUDE all binary files

Grepping through our repo, it seems that this should work

*     text eol=lf
*.py  whitespace=error

*.ico binary
*.jpg binary
*.png binary
*.zip binary

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we even have ico, jpg, png or zip files in our repo?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed

Docs/source/static: favicon & png
Tests/resources: zip
Components/camera: jpg

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's follow the GitHub recommendations including * text=auto

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So could we do * text=lf ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it must be * text eol=lf according to git-scm

It controls different parts:

  • *: Applied to all files, hence the need to remove it on binary files, like images&zip later
  • text: Ensure LF in the repo
  • eol=lf: Attribute to ensure LF in the working directory (This should fix Windows)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So confusing. Okay, so I guess this PR is fine then?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed the version at the top of our discussion, so read to go now

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed confusing, hopefully we can change it once and just forget about it again.... it only really troubles Windows users

/virtualization/Docker/scripts/* text eol=lf
/script/* text eol=lf
*.sh text eol=lf

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no shell or Python scripts in the root? or does this also match **/*.sh ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*.py matches all **/*.sh:

λ git check-attr --all homeassistant\scripts\db_migrator.py
"homeassistant\\scripts\\db_migrator.py": text: set
"homeassistant\\scripts\\db_migrator.py": eol: lf
"homeassistant\\scripts\\db_migrator.py": whitespace: error

*.py text eol=lf whitespace=error