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

Add .gitattributes #2188

Merged
merged 15 commits into from
Sep 13, 2022
Merged

Add .gitattributes #2188

merged 15 commits into from
Sep 13, 2022

Conversation

Zerorigin
Copy link
Contributor

Add .gitattributes to avoid Encoding and EOL formatting changes.

@isaak654 isaak654 added the Confirmation pending Further confirmation is requested label Aug 28, 2022
Copy link
Collaborator

@isaak654 isaak654 left a comment

Choose a reason for hiding this comment

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

1. I think the crlf line ending should not be set for all .bat and .cmd files, but only for specific .bat and .cmd files that might present this issue in future such as RunReport.bat, build.bat, ParseVersion.bat, configure.bat and so on, if you can find other similarities. -> 8c1292f
2. I didn't check them all, but in case make sure to verify that all .bat and .cmd files are already in UTF-8.

@Zerorigin
Copy link
Contributor Author

Zerorigin commented Aug 29, 2022

  1. I think the crlf line ending should not be set for all .bat and .cmd files, but only for specific .bat and .cmd files that might present this issue in future such as RunReport.bat, build.bat, ParseVersion.bat, configure.bat and so on, if you can find other similarities.
  2. I didn't check them all, but in case make sure to verify that all .bat and .cmd files are already in UTF-8.

https://stackoverflow.com/questions/11303405/force-encode-from-us-ascii-to-utf-8-iconv
https://www.shellhacks.com/find-out-text-file-line-endings-lf-or-clrf/

# file $(find . | grep -E "*.(bat|cmd)$")
./Installer/copy_build.cmd:                ASCII text
./Installer/get_openssl.cmd:               ASCII text
./Installer/get_qttranslations.cmd:        ASCII text
./Installer/merge_builds.cmd:              DOS batch file, ASCII text
./Sandboxie/install/build.bat:             DOS batch file, ASCII text
./Sandboxie/install/ParseVersion.bat:      ASCII text
./Sandboxie/msgs/RunReport.bat:            ASCII text
./Sandboxie/msgs/SbieText.bat:             ASCII text
./SandboxiePlus/install_jom.cmd:           ASCII text
./SandboxiePlus/qmake_plus.cmd:            ASCII text
./SandboxiePlus/QtSingleApp/configure.bat: ASCII text
./TestCI.cmd:                              ASCII text


# enca -d -L none $(find . | grep -E "*.(bat|cmd)$")
./Installer/copy_build.cmd: 7bit ASCII characters
./Installer/get_openssl.cmd: 7bit ASCII characters
./Installer/get_qttranslations.cmd: 7bit ASCII characters
./Installer/merge_builds.cmd: 7bit ASCII characters
./Sandboxie/install/build.bat: 7bit ASCII characters
./Sandboxie/install/ParseVersion.bat: 7bit ASCII characters
./Sandboxie/msgs/RunReport.bat: 7bit ASCII characters
./Sandboxie/msgs/SbieText.bat: 7bit ASCII characters
./SandboxiePlus/install_jom.cmd: 7bit ASCII characters
./SandboxiePlus/qmake_plus.cmd: 7bit ASCII characters
./SandboxiePlus/QtSingleApp/configure.bat: 7bit ASCII characters
./TestCI.cmd: 7bit ASCII characters


# enca -m -L none $(find . | grep -E "*.(bat|cmd)$")
./Installer/copy_build.cmd: US-ASCII
./Installer/get_openssl.cmd: US-ASCII
./Installer/get_qttranslations.cmd: US-ASCII
./Installer/merge_builds.cmd: US-ASCII
./Sandboxie/install/build.bat: US-ASCII
./Sandboxie/install/ParseVersion.bat: US-ASCII
./Sandboxie/msgs/RunReport.bat: US-ASCII
./Sandboxie/msgs/SbieText.bat: US-ASCII
./SandboxiePlus/install_jom.cmd: US-ASCII
./SandboxiePlus/qmake_plus.cmd: US-ASCII
./SandboxiePlus/QtSingleApp/configure.bat: US-ASCII
./TestCI.cmd: US-ASCII
  1. At first I simply followed here.

    Sandboxie/.editorconfig

    Lines 31 to 34 in 2a7c51d

    [*.{txt,cmd,bat}]
    charset = utf-8
    end_of_line = crlf
  2. It seems all .bat and .cmd files are the LF line ending and use US-ASCII also call 7bit ASCII or UTF-8?

@isaak654
Copy link
Collaborator

  1. At first I simply followed here.

I’ve changed my mind since then, so I just pushed 8827ece to reflect that.

@Zerorigin
Copy link
Contributor Author

I checked them all again.
I just pushed the new commit to reflect the changes.

for i in $(find . | grep -E "*.(bat|cmd)$")
do
    grep -E "(call|goto)" -l $i
done


./Installer/merge_builds.cmd
./Sandboxie/install/build.bat
./Sandboxie/install/ParseVersion.bat
./Sandboxie/msgs/RunReport.bat
./SandboxiePlus/qmake_plus.cmd
./SandboxiePlus/QtSingleApp/configure.bat
for i in $(find . | grep -E "*.(bat|cmd)$")
do
    export bytes_to_scan=$(wc -c < $i)
    file -P bytes=$bytes_to_scan $i
done


./Installer/copy_build.cmd: ASCII text, with CRLF line terminators
./Installer/get_openssl.cmd: ASCII text, with CRLF line terminators
./Installer/get_qttranslations.cmd: ASCII text, with CRLF line terminators
./Installer/merge_builds.cmd: DOS batch file, ASCII text, with CRLF line terminators
./Sandboxie/install/build.bat: DOS batch file, ASCII text, with CRLF line terminators
./Sandboxie/install/ParseVersion.bat: ASCII text, with CRLF line terminators
./Sandboxie/msgs/RunReport.bat: ASCII text, with CRLF line terminators
./Sandboxie/msgs/SbieText.bat: ASCII text, with CRLF line terminators
./SandboxiePlus/install_jom.cmd: ASCII text, with CRLF line terminators
./SandboxiePlus/qmake_plus.cmd: ASCII text, with CRLF line terminators
./SandboxiePlus/QtSingleApp/configure.bat: ASCII text, with CRLF line terminators
./TestCI.cmd: ASCII text, with CRLF line terminators

@Zerorigin Zerorigin requested review from isaak654 and removed request for DavidXanatos August 30, 2022 13:21
.gitattributes Outdated Show resolved Hide resolved
@isaak654
Copy link
Collaborator

isaak654 commented Aug 30, 2022

🤔 Both main.yml and lupdate.yml are failing with some of the changes (in my forked repo).
And removing the problematic lines right away is not a solution that I would consider.
CI
lupdate

@isaak654 isaak654 marked this pull request as draft August 30, 2022 21:52
@isaak654
Copy link
Collaborator

isaak654 commented Aug 31, 2022

@isaak654 isaak654 requested a review from stephtr August 31, 2022 09:07
@isaak654 isaak654 changed the title Add .gitattributes Add .gitattributes + .gitconfig Sep 5, 2022
@isaak654 isaak654 changed the title Add .gitattributes + .gitconfig Add .gitattributes and .gitconfig Sep 5, 2022
It has only worked in a limited number of cases
@isaak654 isaak654 changed the title Add .gitattributes and .gitconfig Add .gitattributes Sep 7, 2022
@isaak654 isaak654 marked this pull request as ready for review September 7, 2022 19:57
@isaak654 isaak654 requested review from mpheath and removed request for stephtr and mpheath September 7, 2022 20:02
@isaak654 isaak654 removed the Confirmation pending Further confirmation is requested label Sep 13, 2022
@isaak654 isaak654 merged commit cdc0d12 into sandboxie-plus:master Sep 13, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants