chaddro / EasyPopulate-4.0

Data import and export module for Zencart 1.3.x and 1.5.x
GNU General Public License v2.0
24 stars 31 forks source link

missing line end when export product description #28

Open xiezhensheng opened 8 years ago

xiezhensheng commented 8 years ago

When I export full product information, I found the line end is removed from the product description. All my multi-line product description becomes one paragraph.

I found the code deliberately removed the \r \n chars, but not sure exactly why. I understand ms excel has problems to deal with special chars between "". But open office or libre doesn't. And this is very clearly stated in the readme.

This actually make the import/export product information not usable.

I would like to know the consideration here.

Thanks.

in the following code in easypopulate_4_functions.php

function ep_4_rmv_chars($filelayout, $active_row, $csv_delimiter = "^") { // $datarow = ep_4_rmv_chars($filelayout, $active_row, $csv_delimiter); $dataRow = '';

$problem_chars = array("\r", "\n", "\t"); // carriage return, newline, tab foreach ($filelayout as $key => $value) { // $thetext = $active_row[$key]; // remove carriage returns, newlines, and tabs - needs review $thetext = str_replace($problem_chars, ' ', $active_row[$key]); // encapsulate data in quotes, and escape embedded quotes in data $dataRow .= '"' . str_replace('"', '""', $thetext) . '"' . $csv_delimiter; } // Remove trailing tab, then append the end-of-line $dataRow = rtrim($dataRow, $csv_delimiter) . "\n";

return $dataRow; }

change the line: $problem_chars = array("\r", "\n", "\t"); // carriage return, newline, tab to $problem_chars = array("\t"); // carriage tab will just do the job and don't break the csv downloading

it is also interesting to see this line: // remove carriage returns, newlines, and tabs - needs review

mc12345678 commented 8 years ago

So, the function was added when similar code was seen throughout the existing program and to allow similar changes as suggested above to be applied throughout rather than against one specific use or another. I can not speak to the original design of removing the various returns other than it apparently did cause problems for editing the file(s) and therefore was added at that time. I would say that generally speaking such a "return" probably should include the html <br /> or <br> tag which could be applied using the php code nl2br($string) to insert the first of those two before the hard coded "return".

Whether I added the needs review statement or it existed previously, I would say that if I did so it was because it didn't cause me any problems as I use html tags in the content and therefore such "single paragraph" result isn't a problem, but what isn't a problem for me might be a problem for someone else, therefore recognition that perhaps some review would provide some alternate solutions applicable to desired situations. Certainly, one is able to make the changes they see fit once the software is downloaded. :)

As commented, it is something to consider; however, because not everyone uses Open Office as suggested, the issue may have to be addressed in some other way or remain a minor issue at least until other "desirable" code is suggested.

xiezhensheng commented 8 years ago

I see the dilemma here, and different use cases.

I think switch to html based product description is a good idea, not a fix, but a bypass for good reasons.

For now, I am going to leave only \t as the problematic char and to remove when export. As my test, keeping \r and \n in the product description won't break anything. However if I leave \t unfiltered, the export will be broken. To lazy to investigate why...

mc12345678 commented 8 years ago

Understand the "too lazy to investigate" part. :)

As to using one method for export and possibly the same or another for import, have you worked out how those two different directions will act differently if at all?

In response to this I focused primarily on export and am glad you updated the original post. The quoted code only related to SBA and that made the situation confusing but above as provided the situation is clearer.

xiezhensheng commented 8 years ago

The "clear up" code is every where and confusing :) I feel in some of the case there is double "clear up" situation. I think at some point the code needs to be cleared.

I don't have time/resources to test both directions. I feel it is more risky to filter less "problematic words", but this is convenient when I don't want to use complex html editing.

However, to make my website looks pro, I guess I will eventually switch to html based product description.