eidheim / tiny-process-library

A small platform independent library making it simple to create and stop new processes in C++, as well as writing to stdin and reading from stdout and stderr of a new process
MIT License
338 stars 73 forks source link

Fixed examples and tests for windows without MSYS2 (Unicode and multi) #11

Closed marcosbracco closed 8 years ago

marcosbracco commented 8 years ago

Hi, I made some changes to tests and examples for them to run on Windows without MSYS2. On both Unicode and multi-byte. As you can see, on Windows commands like echo or cd are internal to cmd.exe, so you need to launch cmd process. But timeout (somehow equivalent to sleep) is not internal, so no cmd needed. Unicode support requires using windows TEXT() macro for literals to support both cases. And choose between std::to_string or std::to_wstring.

eidheim commented 8 years ago

Thank you again, but I have to give this some thought. The examples and tests are getting a bit ugly, but that is maybe not avoidable.

marcosbracco commented 8 years ago

Yes, I know. I felt the same way. This is the nicer I could do. Maybe a second pair of eyes can improve.

RElesgoe commented 8 years ago

Maybe instead of having the library interface with wide strings, have it interface with UTF-8 encoded narrow strings and internally convert to UTF-16 encoded wide strings for Windows. The user would just write something like Process process3(u8"timeout 5"); instead of Process process3(L"timeout 5");. This would also have a side effect of writing easier code for test units.

marcosbracco commented 8 years ago

If you don't actually need to support unicode, I think you can also just call CreateProcessA instead of plain CreateProcess.

From windows headers:

#ifdef UNICODE
#define CreateProcess  CreateProcessW
#else
#define CreateProcess  CreateProcessA
#endif // !UNICODE

That way you will always be using non-unicode version, regardless of UNICODE being defined or not. You will also not need any #ifdef UNICODE or kind of stuff. Just always non-unicode. I can do it, please confirm you are willing to drop unicode support from your interface.

marcosbracco commented 8 years ago

I have removed unicode support from interface, (back to std::string) and updated tests and examples. On windows this works on both cases, Unicode and MultiByte. I will be out-of-the office for a couple of weeks, so I will not be able to update this.

eidheim commented 8 years ago

As this is somewhat non-critical, I've postponed working on it. Hopefully, I'll come to a conclusion/solution during the following week.

eidheim commented 8 years ago

I decided to not use too many defines in the examples. See the commit above. Thanks for the useful info and testing on Windows without MSYS2!