-
Notifications
You must be signed in to change notification settings - Fork 79
Trim trailing whitespace and ensure text files are POSIX compliant #246
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
base: main
Are you sure you want to change the base?
Conversation
|
With 659 modified files (including SVG and glTF), there's hardly a way to sensibly review this. I think that differentiating between the commit that changes the script, and one or multiple commits for the modified files, it could be clearer (so what exactly was the change to the PHP file that causes all these changes?) But to start with a question waaaay higher: Where and how is this relevant? |
|
@javagl There are many utilities that expect text files to be POSIX-compliant text files, especially on Mac/Linux systems. For example, As for reviewing, click the "Hide whitespace" check box in the "Files Changed" tab to make reviewing easier:
|
| @@ -504,10 +511,10 @@ public function writeReadme ($tagListings=null) { | |||
| $readme[] = $this->metadata['credit'][$ii]; | |||
| } | |||
| //$readme[] = "#### Assembled by " . AppName . ' ' . AppVersion; | |||
| $readme[] = "#### Assembled by " . AppName; | |||
| $readme[] = "#### Assembled by " . AppName . "\n"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 3 total lines of the PHP script changed to specifically add the \n, here is one case.
| 'link'=>'', | ||
| 'text'=>'Cesium Trademark or Logo', | ||
| 'spdx'=>'LicenseRef-LegalMark-Cesium', | ||
| ), | ||
| 'LicenseRef-LegalMark-DGG' => array ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot I included these, I put these changes in here because otherwise the license in .reuse/dep5 gets generated as License: CC-BY-4.0 AND LicenseRef-LegalMark-Khronos AND with a trailing space and a trailing AND. But that should probably get another PR to fix this first. EDIT: So let's merge #248 first.
|
I'm aware of the constant background noise of As a baseline, I'm skeptical for a change like this. So there was something in the The main point is that changing 659 files warrants some scrutiny. This is not C++ code, and when it's only about some IDE inserting an extra line, then this sounds like a configuration issue of the IDE, and not something that warrants such a wide-ranging change. ("Removing trailing whitespaces in |

This PR updates the PHP scripts to output POSIX-compliant text files, which means text files that end in one
\ncharacter. This PR also runs the scripts and manually goes over files the script did not touch (like source.gltffiles). I also added an.editorconfigfile to give a hint to IDEs that they should automatically fix this problem if encountered. This PR also trims trailing whitespace.