ICEcoder v7.0 - Issue with newlines after creating/saving/changing a file
Closed this issue · 15 comments
ICEcoder 7.0 (latest pull from GIT).
Chrome. Latest version; did not change in the last week or so.
Verified behaviour in Edge and Mozilla.
Reproduction:
New File
Add
<?php
echo "something\n";
?>
Save
Close
Re-open, file is now:
<?php\necho "something\n";\n?>\n
If the same file is manually created on the command-line and edited in ICEcoder there are no issues opening, changing saving without a hitch.
Something else I noted: each time you save the file, another \n is added, even if it's blank (zero content).
I can't figure this one out --- curently bypassing by creating the initial file and a few lines with CLI and continue work with ICEcoder from there.
Developer output: zero errors/blank.
HTTP logs: zero errors.
Any thoughts from the top of your mind that may cause this strange behaviour?
No clue where to start to hunt this one down --- happy to pass any info you need to debug/understand.
Anyone that can reproduce this as well?
If the current file is reduced to just 1 line, the same behaviour occurs.
Once corrected via CLI into multiple lines, the issues does no longer re-occur.
Suspect this has something to do with CR vs CRLF detection/processing, but only in a particular case ... perhaps combination of "first save" and "detect if we have CR and/or CRLF in the file"?
Tested on another system. Same issue ...
Some other weird behaviour: cutting/pasting sometimes results in additional copied lines in the source that are NOT visible on screen, but will appear once the file is re-loaded.
Thanks for the great reporting @moudsen - much appreciated. Re the original issue of no line breaks - just trying to understand where the issue may lie - either saving the new file or reading it back.
If you follow your original steps and then view the file via the CLI you also see it without line breaks, and the issue is in saving? The reading back is exactly as you see in the CLI, the saving is the real issue?
Browsing your code changes of the last 2 weeks. Found a possible root cause, but not for certain yet ...
Suspect: the string to file handling is most likely OS based where the classic CR, CRLF and LF discussions.
Foundation for this suspicion: df5d148
Can't think of anything in that commit which would cause it. Must be something not quite right in the writeFile class method: https://github.com/icecoder/ICEcoder/blob/master/classes/File.php#L349-L393
Rolling back commits until hitting a clean version where this is working fine = ce6b124.
Must be something behind this point.... commits up till 0810c18 above this commit do not work. First working commit again is 0f4a90b and this is the point where the behaviour is getting weird handling files.
Your work on July 9th somewhere seems to have introduced the issue?
I'm forcing my version on my systems to ce6b124 in order to be able to work again.
Happy to further investigate with you or do some testing! Will try to dig into the code as well.
Hope this helps ...
I can confirm it is highly likely happening in the WRITE phase when writing the file.
READ works just fine.
Looked at your File.php L349-393 and my best guess is that the code determining EOL is doing something it is not supposed to do:
$contents = str_replace("\r\n", $ICEcoder["lineEnding"], $contents);
$contents = str_replace("\r", $ICEcoder["lineEnding"], $contents);
$contents = str_replace("\n", $ICEcoder["lineEnding"], $contents);
if (isset($_POST['changes']) && (0 < $unixNewLines) || (0 < $windowsNewLines)) {
if ($unixNewLines > $windowsNewLines){
$contents = str_replace($ICEcoder["lineEnding"], "\n", $contents);
} elseif ($windowsNewLines > $unixNewLines){
$contents = str_replace($ICEcoder["lineEnding"], "\r\n", $contents);
}
}
What happens if $ICEcoder["lineEnding"] is empty? How is $ICEcoder["lineEnding"] determined? Is this a new parameter since on July 9th you seem to also have changed the settings code?
Think we are getting closer ...
My best guess at the moment is in the determination of OS based on UnixNewLines or WindowsNewLines as
$contents = str_replace($ICEcoder["lineEnding"], "\n", $contents);
is exactly what is happening ...
Maybe this should be refactored towards your OS detection code and take that as a reference instead of counting \n and \r\n?
Calling it a day - will continue with some debugging tomorrow.
Want to get this fixed because it is annoying and want to stay at your mainstream/head.
Sorry for not helping on investigation a few hours ago - was late night for me when you reported it. Looked into it this morning, found issue after some digging and pushed a commit to fix: a905b0b
Essentially, ICEcoder is now saving global & user config data as serialized data, because the previous method of PHP arrays and string manipulation was fiddly & awkward to work with. In the previous setup we had the lineEnding setting as \n in the PHP array and that's interpreted as a new line character. But in copying config data over to new serialized settings approach I had:
...s:10:"lineEnding";s:2:"\n";...
Looks fine right, but it's not. The \n value is literally interpreted as a backslash and then a n character as a string of 2 characters here. So when it came to replacing line endings it was replacing all actual line endings with a backslash and n, which is why you got <?php\necho "something\n";\n?>\n
To correct this to what it should be you can see in the commit a905b0b I've changed:
s:2:"\n"
to:
s:1:"
"
...so we have an actual \n new line character of 1 character in length (you can see the actual line return between double quotes) rather than 2 characters - a backslash and n.
Totally obscure bug but when spotted and fixed it makes sense, something I messed up when converting to the new serialized config data system.
You were right about the 9th July BTW - that's when I added the serialized file for global config with the \n mistake:
7f02fe9
Let me know if all works OK for you too now - I think it should be all sorted now and hopefully the copy and paste issue you mentioned is resolved also as a follow on to fixing this line ending character issue.
Hi Matt, no need to apologize! I'm very happy that I could help you pinpointing towards the root cause. I will try the new code later today and will share the result. Looking at your explanation I feel confident this is resolved now.
As I said, happy to stay on your latest code, even if that means that I will hit some bugs :-).
It also feels as doing something back for this great tool!
Also inspired me to write some additional coding to help people completely stay of the CLI (like deamon for presenting Apache log data: https://github.com/moudsen/logserverd) to keep our code and systems safe. Also considering writing an article for it later on, including ICEcoder recommendation!
I can confirm the issue has been resolved (for now, LOL).
👍
The idea for the Apache log data sounds good - I've added a note to look into that. If you do write an article I'd be interested in reading it.