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

#16 added std::wstring support for filename. #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andreprager
Copy link

@andreprager andreprager commented Mar 26, 2021

linked to issue: problem reading utf8 filenames on windows

@maxmarsc maxmarsc changed the title added std::wstring support for filename. #16 added std::wstring support for filename. Apr 14, 2021
@maxmarsc maxmarsc linked an issue Apr 14, 2021 that may be closed by this pull request
@maxmarsc
Copy link
Contributor

Hi, thanks for the contribution and the bug hunt, sorry it took us so long to review it.

Unfortunately the linux build seems to be broken (we should definitively add CI for linux build). We don't have time to investigate right now but here is the error :

Scanning dependencies of target wave
[ 47%] Building CXX object src/wave/CMakeFiles/wave.dir/header/data_header.cc.o
[ 52%] Building CXX object src/wave/CMakeFiles/wave.dir/header/riff_header.cc.o
[ 57%] Building CXX object src/wave/CMakeFiles/wave.dir/header/fmt_header.cc.o
[ 63%] Building CXX object src/wave/CMakeFiles/wave.dir/header/wave_header.cc.o
[ 68%] Building CXX object src/wave/CMakeFiles/wave.dir/header.cc.o
[ 73%] Building CXX object src/wave/CMakeFiles/wave.dir/header_list.cc.o
.../wave/src/wave/header_list.cc: In member function ‘wave::Error wave::HeaderList::Init(const wstring&)’:
.../wave/src/wave/header_list.cc:45:38: error: no matching function for call to ‘std::basic_ifstream<char>::open(const wstring&, const openmode&)’
   stream_.open(path, std::ios::binary);
                                      ^
In file included from .../wave/src/wave/header_list.h:4:0,
                 from .../wave/src/wave/header_list.cc:1:
/usr/include/c++/7/fstream:595:7: note: candidate: void std::basic_ifstream<_CharT, _Traits>::open(const char*, std::ios_base::openmode) [with _CharT = char; _Traits = std::char_traits<char>; std::ios_base::openmode = std::_Ios_Openmode]
       open(const char* __s, ios_base::openmode __mode = ios_base::in)
       ^~~~
/usr/include/c++/7/fstream:595:7: note:   no known conversion for argument 1 from ‘const wstring {aka const std::__cxx11::basic_string<wchar_t>}’ to ‘const char*’
/usr/include/c++/7/fstream:615:7: note: candidate: void std::basic_ifstream<_CharT, _Traits>::open(const string&, std::ios_base::openmode) [with _CharT = char; _Traits = std::char_traits<char>; std::__cxx11::string = std::__cxx11::basic_string<char>; std::ios_base::openmode = std::_Ios_Openmode]
       open(const std::string& __s, ios_base::openmode __mode = ios_base::in)
       ^~~~
/usr/include/c++/7/fstream:615:7: note:   no known conversion for argument 1 from ‘const wstring {aka const std::__cxx11::basic_string<wchar_t>}’ to ‘const string& {aka const std::__cxx11::basic_string<char>&}’
src/wave/CMakeFiles/wave.dir/build.make:127: recipe for target 'src/wave/CMakeFiles/wave.dir/header_list.cc.o' failed
make[2]: *** [src/wave/CMakeFiles/wave.dir/header_list.cc.o] Error 1
CMakeFiles/Makefile2:1081: recipe for target 'src/wave/CMakeFiles/wave.dir/all' failed
make[1]: *** [src/wave/CMakeFiles/wave.dir/all] Error 2
Makefile:140: recipe for target 'all' failed
make: *** [all] Error 2

If you can build on linux please try to fix it, otherwise tell us and we'll try to look into it when we have the time.

Furthermore, please add some tests of your new methods :

  • new File constructor should be tested in src/wave/file_test.cc
  • new Header Init method should be tested in src/wave/header_test.cc

@andreprager
Copy link
Author

It seems the std::wstring interface for std::istream is an extension of MSVC, I will check for a workaround.

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

Successfully merging this pull request may close these issues.

problem reading utf8 filenames on windows
3 participants