Php diff 120 - #128
Conversation
DigiLive
commented
Nov 19, 2024
Hey, Of course I'm willing to do a review for you, but you'll have to grant me some time. Aside from work, I'm also active in some other projects which require my attention. |
JBlond
commented
Nov 19, 2024
You don't need to worry about time. Do it whenever you find time. |
There was a problem hiding this comment.
Assumming you'll still need to cleanup/format the code, but...
- The files (ex. the eol resources) have an incorrect line seperator.
- Docblocks are missing or unfinshed.
- Since the endings don't change after the file is loaded, you could execute the different methods immediately after loading a file, store the values as properties and use getters for these properties.
Also, I left in-file comments.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| { | ||
| $lastLine = $this->getLastLine(); | ||
| if (strpos($lastLine, "\r\n") !== false) { | ||
| return "EOL type is Windows (CRLF)"; |
There was a problem hiding this comment.
Double quotes can be replaced by single quotes
There was a problem hiding this comment.
it can't be replaced. if I replace it with single quotes the unit test fails.
There was a problem hiding this comment.
The comment is referring to line 72, not 71.
| } | ||
| if (strpos($lastLine, "\r") !== false) { | ||
| return "EOL type is Mac (CR)"; |
There was a problem hiding this comment.
Double quotes can be replaced by single quotes
There was a problem hiding this comment.
it can't be replaced. if I replace it with single quotes the unit test fails.
There was a problem hiding this comment.
The comment is referring to line 76, not 75.
| } | ||
| if (strpos($lastLine, "\n") !== false) { | ||
| return "EOL type is Unix (LF)"; |
There was a problem hiding this comment.
Double quotes can be replaced by single quotes
There was a problem hiding this comment.
it can't be replaced. if I replace it with single quotes the unit test fails.
There was a problem hiding this comment.
The comment is referring to line 80, not 79.
DigiLive
commented
Jan 27, 2025
Also, If I'm not mistaken, I notice now, the line separators ( |
No description provided.