Refactored Get-Associated-Icon function to handle file path errors

Enubia commented 4 months ago

If the executable name contains special characters the PowerShell script will crash because it cannot handle the file path correctly.

Before before

After after

oliverschwendener commented 4 months ago

Thanks for reporting this issue, I wasn't aware of this.

I think in this case the issue is not the icon extraction, although that can be improved as well. It seems like when parsing the output of the powershell command, those umlauts aren't interpreted correctly.

Here is my example: In the start menu folder, I created a shortcut with an umlaut, see this screenshot:

The WindowsApplicationRepository invokes a powershell script to read all *.lnk files in the start menu folder and parses its ouput. Here it's how it's parsed:

{
  Name: 'Visual Studio Code∩┐╜.lnk',
  FullName: 'C:\\Users\\Oliver\\AppData\\Roaming\\Microsoft\\Windows\\Start Menu\\Programs\\Visual Studio Code\\Visual Studio Code∩┐╜.lnk',
  Extension: '.lnk',
  BaseName: 'Visual Studio Code∩┐╜'
}

The filepath of FullName will then be passed to the WindowsApplicationIconExtractor which then invokes the second powershell script to extract the icons. I think to solve this problem, we have to see that the first powershell script output is encoded/decoded correctly. I'll look into it in the next days.

Enubia commented 4 months ago

This was also my first guess but since I'm not that proficient in powershell scripting I opted for an easy and quick solution to just try catch the crash and let the defaults take precedence.

Of course the best solution would be fixing the initial name parsing and interpret the text in presumably the right encoding?

oliverschwendener commented 4 months ago

I noticed that even when using node's fs.readdir function to get the shortcut files, it will interpret the umlaut characters in the file name incorrectly. When using node instead of electron it work as expected though, see this repo here: https://github.com/oliverschwendener/nodejs-readdir-test. I'm not sure what the problem is.

Enubia commented 4 months ago

I noticed that even when using node's fs.readdir function to get the shortcut files, it will interpret the umlaut characters in the file name incorrectly. When using node instead of electron it work as expected though, see this repo here: oliverschwendener/nodejs-readdir-test. I'm not sure what the problem is.

yup, just checked it myself and indeed the encoding is messed up. I even tried to force the encoding via an optional parameter and it still didn't work.

oliverschwendener commented 4 months ago

I reported a bug in electron here: https://github.com/electron/electron/issues/42035

oliverschwendener commented 4 months ago

Nevertheless, I'll merge this PR as it solves the issue of not crashing the whole icon extraction just because of one file. Now it just shows a broken icon like this, which is fine for now:

Thanks alot for your time and the efforts!

oliverschwendener commented 4 months ago

For next time, please make sure you set up commit singing as it will be required for future contributions, thanks!

oliverschwendener / ueli

Refactored Get-Associated-Icon function to handle file path errors #1096