Skip to content

Conversation

@yumetodo
Copy link
Contributor

@yumetodo yumetodo commented Mar 4, 2018

msys2のgcc(clangはだめ)でもビルドできたらいいなと思い対応してみた。

Copy link
Contributor Author

@yumetodo yumetodo left a comment

Choose a reason for hiding this comment

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

大体C++標準ではstd::ofstreamのctorにwchar_t系の文字列渡せないのが問題。

return 1;
}
std::ifstream target(path, std::ios::binary);
std::ifstream target(path.string(), std::ios::binary);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

文字コード変換をサボって雑にやっている。日本語パスが来た時にどうなるのか怖い

if (information_class == FileRenameInfo) {
auto& info = *static_cast<PFILE_RENAME_INFO>(file_information);
std::ofstream(info.FileName, std::ios::ate | std::ios::binary);
std::ofstream(fs::path(info.FileName).string(), std::ios::ate | std::ios::binary);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

文字コード変換をサボって雑にやっている。日本語パスが来た時にどうなるのか怖い

@yumetodo
Copy link
Contributor Author

yumetodo commented Mar 4, 2018

現在msys2のpacmanで拾えるgccは7.3.0だけれども、たしかgcc8.0.0からfilesystemがexperimentalではなくなるので、そのときは 78af762, 77e6c8a に改修が必要。

@kazatsuyu
Copy link
Owner

http://en.cppreference.com/w/cpp/io/basic_fstream/basic_fstream
std::experimentall::filesystemを使っている時点でどうなのという話ではあるが、このプロジェクトはC++17以前の環境をターゲットにしておらず、C++17ならstd::filesystem::pathconst std::filesystem::path::value_type*を受け取れるので、日本語パスの扱いが不安なコードはあまり入れたくない。
少なくともこのコードは、MSVCの環境ならCP932になってしまうので、入れるなら日本語パスに関するテストを追加した上で対象の環境を切り分けて欲しい。

@yumetodo
Copy link
Contributor Author

yumetodo commented Mar 5, 2018

正直な話Win32APIないしboostで書き換えたい。filesystem使うには時期尚早。そもそもmingwのwchar_t周りは大体壊れていて地雷原を進むようなものなのでgcc8.0.0が来てもまともに実装されていると思えず、Win32API以外で使いたくない。

@kazatsuyu
Copy link
Owner

規模これ以上は大きくならないだろうと思われるためboostには依存したくない。
ターゲットがWindows以外になり得ないためWin32 APIに依存するのは構わない。

cmake_minimum_required(VERSION 3.10.0)
enable_language(CXX)
set(CMAKE_CXX_STANDARD 17) # C++17...
set(CMAKE_CXX_STANDARD_REQUIRED ON) #...is required...
Copy link
Owner

Choose a reason for hiding this comment

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

この方法に変えてしまうと非mingw環境で動かないので戻してください

Copy link
Contributor Author

Choose a reason for hiding this comment

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

どう動かないのでしょう?

Copy link
Owner

Choose a reason for hiding this comment

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

CMakeの-Gオプションで"Visual Studio 15 2017" を指定した場合に set(CMAKE_CXX_STANDARD 17) だと-std=c++17もしくは/std:c++17オプションが付かない。原因を詳しく調査はしていないが、CMAKE_CXX_FLAGSを使っているのはそのため。回避策が分かるなら変更してもいい

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.

2 participants