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

fix: put back LF (unix) line separators in csv exports #447

Merged
merged 1 commit into from
Apr 23, 2020

Conversation

jcharlet
Copy link
Contributor

@jcharlet jcharlet commented Apr 16, 2020

Issue

Fix #440

Context

this is new in 6.5.

This is due to a change in the library used to create CSV format (done by @adamretter to improve processing speed).
The library by default outputs files in a different format according to the operating system (using the operating system's own convention), while the previous one just used Unix format.

https://github.com/uniVocity/univocity-parsers/blob/master/src/main/java/com/univocity/parsers/common/Format.java
 *  <li><b>lineSeparator:</b> the 1-2 character sequence that indicates the end of a line. Newline sequences are different across operating systems. Typically:
 *		<ul>
 *			<li>Windows uses carriage return and line feed: <i>\r\n</i></li>
 *			<li>Linux/Unix uses line feed only: <i>\n</i></li>
 *			<li>MacOS uses carriage return only: <i>\r</i></li>
 *		</ul>
 *   	<i>{@link Format#lineSeparator} defaults to the system line separator</i>
 *  </li>

Solution

manually set the CSV format back to Unix.

Acceptance tests

  • start GUI in Windows
  • run analysis on random files
  • export CSV
  • verify that CSV files use LF (Unix) line separating instead of CRLF (Windows)

@jcharlet jcharlet added the bug label Apr 16, 2020
@jcharlet jcharlet added this to the 6.5 milestone Apr 16, 2020
@jcharlet jcharlet requested a review from sparkhi April 16, 2020 11:51
@jcharlet jcharlet self-assigned this Apr 16, 2020
@jcharlet jcharlet added this to In progress in DROID Kanban board 6.5 via automation Apr 16, 2020
DROID Kanban board 6.5 automation moved this from In progress to Review in progress Apr 16, 2020
Copy link
Contributor

@adamretter adamretter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire file seems to have been reformatted? I can't spot the change :-(

@jcharlet
Copy link
Contributor Author

argh sorry @adamretter , you can ignore whitespace changes.
Screenshot from 2020-04-16 15-32-03

will undo that while fixing the checkstyle issue.

@jcharlet
Copy link
Contributor Author

meh.. intellij doesn't see the space change :(
Screenshot from 2020-04-16 15-35-23

@jcharlet jcharlet force-pushed the bug_endline_440 branch 3 times, most recently from df8a784 to 49d3951 Compare April 16, 2020 17:52
@jcharlet
Copy link
Contributor Author

@adamretter finally understood what the issue was.. and funny enough, it was the way my IDE was formatting the file using.... Unix LF instead of Windows CRLF, which changed the whole files. after a big of fighting with it, I fixed it, checkstyle issues and unit tests on windows. Should be fine now.

@adamretter
Copy link
Contributor

Are you using Intellij? If so it normally follows whatever your git repo is configured for I think

@jcharlet
Copy link
Contributor Author

yes but for some reason if there was already files staged, it wouldn't see the change when I update the file format (with intellij or unix2dos shell command).
Can't update your requested changes @adamretter cause I overwrote my commit to keep it clean, but it should be fixed now and ready for review.

@jcharlet jcharlet merged commit ef93aa6 into master Apr 23, 2020
DROID Kanban board 6.5 automation moved this from Review in progress to Done Apr 23, 2020
@jcharlet jcharlet deleted the bug_endline_440 branch April 23, 2020 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

profiles produced now have Windows ends (CR+Newline) rather than just Unix (Newline)
3 participants