free-audio / clap

Audio Plugin API
https://cleveraudio.org/
MIT License
1.76k stars 97 forks source link

Win: Correct non-existing environment variable and native directory separator #204

Closed Schroedingers-Cat closed 1 year ago

Schroedingers-Cat commented 1 year ago

This PR improves two things in the entry.h file's comment section:

abique commented 1 year ago

So it seems that %CommonFilesFolder% is for MSI installer: https://learn.microsoft.com/en-us/windows/win32/msi/commonfilesfolder

abique commented 1 year ago

Windows accepts both / and \ as directory separator isn't it?

abique commented 1 year ago

https://learn.microsoft.com/en-us/windows/deployment/usmt/usmt-recognized-environment-variables

mkruselj commented 1 year ago

PR looks good to me! Windows does seem to accept both directory separators, but it's still using \ pretty much everywhere throughout its interface.

abique commented 1 year ago

Could it be that we provide both the MSI installer variables and environment variable in the documentation? Or would it be misleading?

mkruselj commented 1 year ago

Not everyone would be writing a MSI installer (there's InnoSetup, InstallShield, NSIS...) so I think it's better to use the system envvar here, it's super clear then and developers can consult their installer's documentation for additional clarification (if they already don't know these things by heart).

abique commented 1 year ago

Then we're good to go I believe.

robbert-vdh commented 1 year ago

Windows accepts both / and \ as directory separator isn't it?

Pretty much everywhere except for inside of the GUI file open dialog. So for actual cross-platform code it can be a good idea to just use forward slashes everywhere to reduce the amount of platform-specific bits mixed in with the rest of the code.