Function for opening default browser with a given URL #154
Conversation
|
Here is the original commit, including the related discussion: 8fef7af |
| import core.sys.windows.windows; | ||
|
|
||
| extern (Windows) | ||
| HINSTANCE ShellExecuteA(HWND hwnd, LPCSTR lpOperation, LPCSTR lpFile, LPCSTR lpParameters, LPCSTR lpDirectory, INT nShowCmd); |
There was a problem hiding this comment.
Can ShellExecuteA really handle UTF-8 arguments? Why not use ShellExecuteW?
There was a problem hiding this comment.
It's for opening websites, websites addresses don't come in UTF8 as far as I know? Or has this changed? I'm completely out of the loop when it comes to internet rules. RDMD uses it for:
ShellExecuteA(null, "open", "http://www.digitalmars.com/d/2.0/rdmd.html", null, null, SW_SHOWNORMAL);
There was a problem hiding this comment.
As of now, there are domain names in the native language, though I dunno what kind of unicode they are supposed to use.
There was a problem hiding this comment.
@blackwhale You are probably referring to Punycode – but I don't know off-hand if it would happen before ShellExecute or if it would have to be manually applied beforehand (I suspect the latter, but that's a pure guess).
There was a problem hiding this comment.
Take a look at this: http://stackoverflow.com/questions/155892/unicode-url-decoding
It looks like the truly correct way to handle this is to take unicode and then encode the non-ASCII characters per the standard. And that goes for all OSes.
Regardless, I believe that the general rule with regards to the Windows functions is that the W version should be used (perhaps with a fallback to the A version on older Windows systems). We really shouldn't be using the A versions.
|
Okay. The real question here is who is going to do the work on this? It's Walter's code, but it's my pull request. he hasn't had anything to say about it or about the feedback on the pull request. Should I just go ahead and alter it as I see best based on the feedback here? |
|
Yah, sorry for making it look like you've been framed :o). Let me merge this as is and we'll leave the task up for grabs. |
Function for opening default browser with a given URL
|
Task up for grabs that's hidden off in a closed pull request. Yeah, never gonna happen. If you feel they're important, do them now or file a bug report so at least there's a chance of being found. |
|
It will happen, as Phobos modules are under constant scrutiny. I know I do make a pass once in a while. |
|
I think that it would have been better to just ask me to deal with it or have someone else pull in the changes from my repo, do whatever fixes are necessary and then either create a new pull request or ask me to pull them to fix this pull request. Yes, Phobos is under constant scrutiny, but stuff still falls by the wayside. As it is, this risks not getting fixed before the next release, and we release code which we haven't fixed when we have feedback with regards to what about it should be fixed. |
…-file rdmd: Add --extra-file option
Okay. This pull request is on Walter's behalf. I reverted the original commits and created this pull request as discussed. It was suggested that it was better to put the function in std.process instead of its own module, so that's what I did. The only changes that I made were to remove some imports that std.process was already importing and fully give the full module path for
getenvand andexecvp, since Walter used the C versions rather than those in std.process. Personally, I think that the code should be changed to use the std.process ones instead, but I didn't make any such functional changes prior to creating the pull request, since it's not my code, and it should probably be reviewed as Walter created it (which may very well in it getting changed as I suggested).