Unicode improvements and fixes #1044
Conversation
|
@AlexHarker I have merged cockos/main to master, can you rebase this branch? |
|
I will rebase before I merge - it can be automatically rebased at the moment, so it should stay that way and there will be a nice clean history as a result (but at the moment I'm still testing specific functionality to avoid messing up windows builds) |
|
It shouldn't make a difference to the history. it will be easier to review the PR if it just has relevant commits |
|
(would like to review it when ready) |
AlexHarker
force-pushed
the
unicode-improvements-and-fixes
branch
2 times, most recently
from
232aaa1
to
b1bf8b8
Compare
|
There are still some minor things to look at and discuss but on the whole this is done and tested - it is now ready for review @olilarkin and has been rebased. |
|
@svantana - if you had a moment to check out this branch and double check that SetFilePathInClipboard() is working on windows satisfactorily that would be great. I have tested, but it looks like you originally contributed this method and you are likely to have more realworld tests for it. We are particularly interested in paths with non-ascii characters. You might like to try file dropping also as you added the multiple file drop stuff. |
olilarkin
force-pushed
the
unicode-improvements-and-fixes
branch
3 times, most recently
from
4f2b00b
to
cae627a
Compare
There was a problem hiding this comment.
This is a great improvement!
I took the liberty to "fixup" the commits where you missed something into the commits where the change originally belonged (git commit --fixup SHA is a favorite in the day job)
I also corrected some typos in the commit messages.
I have tried out a few things and couldn't spot anything that has broken. I haven't thoroughly reasoned about the code changes but I trust your work and if there is a problem in anything we can fix what is already a big improvement on what was there.
We don't really have the time to be uber-disciplined or nitpicky in our git history/PRs, but I think we should strive towards a better system. Eventually I hope we can get everything clang-formatted and have git hooks to enforce code style and solve all the inconsistencies w.r.t whitespace etc.
One thing that would make this PR easier to review is if it was split up into a couple of PRs. There are a bunch of commits that relate to the APP wrapper, and a bunch that relate to IGraphicsWin, along with one or two that relate to other things like IPlugFaustGen. Sometimes the commit message is not very explicit about which thing is getting updated. Sometimes I put e.g. APP: at the beginning of commit messages todo with the app wrapper.
So could I ask you to rebase it, pulling all the APP related unicode changes into a 2nd PR? Maybe even a couple of other PRs for e.g. IPlugFaustGen.h changes?
I've become a big fan of interactive rebasing to try and tell a nice story with the commits. It is good if the meat of the PR comes at the end and any cleanups are done at the beginning.
I am also happy to make these changes and merge if you are busy
… mac (leave trailing spaces and do not add carriage returns etc.)
…ng character (for an empty string even)
olilarkin
force-pushed
the
unicode-improvements-and-fixes
branch
from
cae627a
to
351fdea
Compare
|
@AlexHarker This is now cleaned up as discussed. APP related commits are now in #1054 , IPlugFaustGen.cpp change extracted. I think this should be mergeable now... let me know |
|
I am confident that this represents the correct split (I've spent some time checking difference and reconstructing any formatting edits to be confident that everything is there, and that each part of the split is correct. There's something potentially missing in the Faust changes, but it is minor and looks like it was never committed - that would be fairly low priority but relates to the DLL loading on windows. |
cool. i will look at faust separately i think |
This PR replaces #1006 and is focused on improving unicode support for windows.
In general iPlug2 treats strings as UTF8, but this is not natively supported under windows, so conversion between UTF8 and UTF16 is needed to account for the changes. Here we explicitly call windows functions in their wide version (ending W()), rather than the ANSI version (ending A()) and perform manual conversion. We avoid the macro versions that switch based on the settings of UNICODE and _UNICODE. This makes settings relating to unicode (or macro definitions) irrelevant to which functions iPlug2 makes use of, but leaves developers free to use that for other purposes.