Fixes various small bugs with the quote and separator choices #611
Conversation
justinclift
added
the
bug
|
A little off-topic regarding this PR, but, does this project have a coding style? I noticed that you put the brackets on the same line (I personally prefer this too), while in the rest of the project they are on a separate line. I think it would be a good idea to stick with a single coding style throughout the entire project. What do you think? Maybe we ca discuss this in more depth in #602. |
|
Hmmm, I've seen both styles of bracket placement in the code already. (eg do a grep -r "else" from the main dir) So, I just picked my preference. 馃槃 |
|
After sleeping on it, I kind of think the switch statement approach used in currentNewLineString() is super clear and without unwanted side effects: https://github.com/sqlitebrowser/sqlitebrowser/blob/master/src/ExportCsvDialog.cpp#L322-L334 So, I might try redoing some of the methods in this PR using that, and see if it's an improvement. 馃槃 |
|
I personally indent the |
|
My question wasn't necessarily about this PR, it was about the whole project and the future PRs. But it was just an idea 馃槃 |
|
Yeah, its a good idea. 馃槃 |
|
With indenting the cases... I actually remember my decision here. I went with the default that Qt Creator set them to when I first entered the switch keyword and hit return. Personally, it's not something I care much about, so if we decide to have a defined coding style for them I'm happy to do whatever. 馃槃 |
Expanded out many of the functions so they can be single stepped
through easily by newbies (eg me), which allowed for identifying
(then fixing) a few minor bugs.
* if setSeparatorChar() had an empty value passed to it the first
time, then in subsequent calls it would silently use \0 instead,
including using that in the output CSV data. Likely not great. ;)
* the empty option for the quote separator is now correctly retained
between runs of the dialog
|
Rewrote the functions using a switch statement. Also fixed another minor-ish bug in the output, found when examining the csv output using hexedit. Side note, that hexedit util is nifty. Nicely, it's available via Homebrew ( |
Fixes #610.
Expanded out many of the functions so they can be single stepped
through easily by newbies (eg me), which allowed for identifying
(then fixing) a few minor bugs.
time, then in subsequent calls it would silently use \0 instead,
including using that in the output CSV data. Likely not great. 馃槈
between runs of the dialog