Closed chrisgleissner closed 5 years ago
You're right, this is quite a simple fix. I will get this out when I can. Until then, I encourage you to convert the null values on your side. Thank you for using the library and thank you for the report!
Hi Mitch,
Thanks for your reply. If I fix it as part of a merge request, would you consider merging it? I wanted to check first before I start work on it.
Thanks Christian
On Sat, 3 Nov 2018, 08:20 Mitch Talmadge <notifications@github.com wrote:
You're right, this is quite a simple fix. I will get this out when I can. Until then, I encourage you to convert the null values on your side. Thank you for using the library and thank you for the report!
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MitchTalmadge/ASCII-Data/issues/11#issuecomment-435569789, or mute the thread https://github.com/notifications/unsubscribe-auth/ADyQe_-MBvn1LyAscSWwE2ULVlvkx2D7ks5urVG2gaJpZM4YMOk_ .
Certainly I would, and this would help me too with my busy schedule. Thank you!
Great. I already implemented it and will raise a pull request this evening.
On Sat, 3 Nov 2018, 09:15 Mitch Talmadge <notifications@github.com wrote:
Certainly I would, and this would help me too with my busy schedule. Thank you!
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MitchTalmadge/ASCII-Data/issues/11#issuecomment-435573080, or mute the thread https://github.com/notifications/unsubscribe-auth/ADyQe5lmE0Mciq0MDJZgS04N8FjjhfVtks5urV7EgaJpZM4YMOk_ .
I raised the PR. My local build passes, but the PR's automatic build fails due to a key issue. The changes were quite simple. It would be great if you could have a look and cut a new Maven Repository release if you like them as I need the changes for a project I am working on. Thanks
https://travis-ci.org/MitchTalmadge/ASCII-Data/builds/450411245
Thank you! The error is because you committed to master instead of develop (my bad, I didn't warn you) and so Travis has some protections in place to prevent decryption on external PRs. It's not a problem though and I will merge the request.
Released as 1.3.0. Let me know how it goes. Thank you for the quality code and I really appreciated that you wrote test cases. :)
Hi Mitch,
Thanks, that makes perfect sense. I had been wondering about your dev branch. Thanks for merging.
Best wishes Christian
On Sun, 4 Nov 2018, 07:29 Mitch Talmadge <notifications@github.com wrote:
Thank you! The error is because you committed to master instead of develop (my bad, I didn't warn you) and so Travis has some protections in place to prevent decryption on external PRs. It's not a problem though and I will merge the request.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MitchTalmadge/ASCII-Data/issues/11#issuecomment-435648401, or mute the thread https://github.com/notifications/unsubscribe-auth/ADyQey71_ia90yw-IwUFIjzJdMc1Tcu6ks5urpdAgaJpZM4YMOk_ .
An NPE is thrown when some of the data cells are null. It would be good if these could be rendered in a special way (e.g. as "null" or configurable via a new method
withNullValue(String)
) to differentiate them from empty strings.Example to reproduce the NPE:
This results in:
I appreciate that I could convert the data before passing it into ASCIITable, but having absent values is quite a common scenario and I think your library would be improved by handling this gracefully.