Closed yumetodo closed 8 years ago
理由すら書かれずにreject食らうとは思わなんだ。
That is mainly my bad... because I've introduced a LOT of changes making some of your patch incompatible. Notably, I've dropped multilingual UI support in exchange for multiple plugins in a single AUF. VS Project file path have been fixed as you mentioned, but I'll have to reject the idea of introducing a whole external library just for checking codepage, and no to a change in plugin name from anyone except me... for instance, you can try submitting a "translation" for FFmpeg's name in Japanese and request to change all instance of "FFmpeg" to your translation... imagine what would happen...
More on the code page stuff. I really don't get your point for finding the code page manually from the registry... there are a number of potential problems for that approach:
Also, when two approaches can achieve the same thing with similar performance, the simpler method would be preferable.
That is also why I don't appreciate your obsession with the newest C++ language standard... when something simple can be achieved using C-like style and not really bad readability, why bother to pull in the whole STL or template...
That is also why I don't appreciate your obsession with the newest C++ language standard... when something simple can be achieved using C-like style and not really bad readability, why bother to pull in the whole STL or template...
OK, I got it. However, I still think that RegOpenKeyEx
and RegQueryValueEx
shoud be wrapped by C++-like interface because of error handling.
User can CHANGE locale on LAUNCH, for example, by using AppLocale. That's why Win32API has all those "Thread Locale" and "OEM Locale"...and so on.
RegOpenKeyEx
and RegQueryValueEx
is thread safe, and microsoft::win32::registry_key
object in function GetFilterTable was not shared with other thread. As a result, the code was totally thread-safe.
Who can guarantee the registry path will not change?
Who can guarantee the Win32API's behavior will not change?
Thread safety: In the case of AviUtl plugin loading, it doesn't have any significant. The filter table will be thrown out ONCE on AviUtl startup.
Win32API wrapper: than according to your logic, you should make your own Win32API-C++. Again, ask yourself is it ESSENTIAL.
Win32API changed only little since WinXP, mostly enhancement with few deprecation. For basic function like locale, MS is not going to change it, at least not intentionally. They may accidentally break it, but in such case, that would break a lot of things.
I'm not a language purist and I'm happy to mix C and C++ style for my convenience.
Win32API wrapper: than according to your logic, you should make your own Win32API-C++. Again, ask yourself is it ESSENTIAL.
Not ESSENTIAL but important.
Of course you know that there is no string type and no exceptions in C. So, we need to manage memory and write many if
statement even if I just want to get string. And we sometime produce bug around memory management and error handling like this.
We must not (cannot?) lock registry while trying to read it. Therefore, we need to write loop to get string via RegQueryValueEx
(watch here)
So, I think that every C-API which is for getting string should be wrapped using std::basic_string
.
As a result, I sometimes produce wrapper like https://github.com/yumetodo/windows_registry_manager
BTW, the main reason of rejecting this pull request is
I've dropped multilingual UI support in exchange for multiple plugins in a single AUF.
and I agree. In this case, multilingual UI support is not important.
SYSTEM\CurrentControlSet\Control\Nls\CodePage\OEMCP
をみるように。https://github.com/yumetodo/windows_registry_manager
をsubmoduleとして追加(header-only library)