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

missing line end when export product description #28

Open
xiezhensheng opened this issue May 8, 2016 · 4 comments
Open

missing line end when export product description #28

xiezhensheng opened this issue May 8, 2016 · 4 comments

Comments

@xiezhensheng
Copy link

xiezhensheng commented May 8, 2016

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants