search

allen-cell-animated / agave

Other
35 stars 6 forks source link

more robust method to get windows log dir #221

Closed toloudis closed 3 weeks ago

toloudis commented 1 month ago

Est review time: ~10 min

A Windows user was having trouble launching AGAVE and one of the only things that can go wrong at startup is the failure to initialize the app logging system.

This code change uses a more robust way of getting a log directory on Windows, using a special Windows system function call.

toloudis commented 4 weeks ago

Looks good from where I'm sitting... I'm on my Mac, reviewing Windows specific code that I don't understand. 👍 But it's always educational to review your PRs, learning about string encoding today.

I could try to install this on a Windows computer I have at home if you want more detailed testing, but otherwise I won't stand in the way of merging this if you think its a good idea.

I have tested it on a couple different Windows machines and it came up with the same log directory as before, which is good. The key difference was relying on an environment variable before, vs now using the windows system call. And, Windows string handling is a bit nuts so I'm lucky it was even this easy with the ConvertBSTR library function.