handle spaces in image file names#4059
Conversation
|
Can you add a unit test that verifies this behavior? Also, before this can be merged, it should be squashed into a single commit. |
|
sure. just found that CI failed :( |
|
I add a unit test and a new image in examples. |
|
Thanks for the test case; LGTM! |
| LOG(INFO) << "Opening file " << source; | ||
| std::ifstream infile(source.c_str()); | ||
| string filename; | ||
| std::string line; |
There was a problem hiding this comment.
Why did we switch from string to std::string here? If we're going to using things (and we do using std::string in common.hpp), then I'd rather consistently drop the namespace.
There was a problem hiding this comment.
sorry I didn't notice that we're using std::string, I'll remove std::
|
Just added some minor stylistic comments, and then I'd like to proceed with this PR; #3433 is essentially similar, but this one fixes some issues there; we'll credit @bchu for the original implementation in the merge message. #1971 is more complex and not presently ready for merge, so let's wait to consider a possible future rebase of that. |
| std::ofstream spacefile(filename_space_.c_str(), std::ofstream::out); | ||
| LOG(INFO) << "Using temporary file " << filename_space_; | ||
| spacefile << EXAMPLES_SOURCE_DIR "images/cat.jpg " << 0 << std::endl; | ||
| spacefile << EXAMPLES_SOURCE_DIR "images/cat gray.jpg " << 1 << std::endl; |
There was a problem hiding this comment.
As a side note (don't fix it here): it seems awkward to me to keep test-dependent files in the examples/ directory...
|
This looks good, thanks for addressing this long-standing issue! |
|
Could you close this issue: #1935? |
handle spaces in image file names Thanks @bchu for an earlier implementation.
4 participants
No description provided.