Customize download folder location and fix Windows autodetection#27472
Customize download folder location and fix Windows autodetection#27472julien-wff wants to merge 2 commits intoraycast:mainfrom
Conversation
|
Thank you for your contribution! 🎉 🔔 @xmok you might want to have a look. You can use this guide to learn how to check out the Pull Request locally in order to test it. 📋 Quick checkout commandsBRANCH="ext/immich"
FORK_URL="https://github.com/julien-wff/raycast-ext-immich.git"
EXTENSION_NAME="immich"
REPO_NAME="raycast-ext-immich"
git clone -n --depth=1 --filter=tree:0 -b $BRANCH $FORK_URL
cd $REPO_NAME
git sparse-checkout set --no-cone "extensions/$EXTENSION_NAME"
git checkout
cd "extensions/$EXTENSION_NAME"
npm install && npm run devWe're currently experiencing a high volume of incoming requests. As a result, the initial review may take up to 10-15 business days. |
Greptile SummaryThis PR adds a user-configurable Confidence Score: 3/5Not safe to merge as-is — the core Windows registry detection is silently broken due to an untrimmed PowerShell output string. A single P1 bug directly undermines the main goal of the PR (fixing Windows download folder detection); the fix is a one-line extensions/immich/src/utils/download.ts — specifically the Important Files Changed
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/immich/src/utils/download.ts
Line: 23-32
Comment:
**`runPowerShellScript` output not trimmed — `existsSync` will always return false**
`runPowerShellScript` returns raw stdout, which includes a trailing newline (`\r\n` on Windows). Passing that untrimmed string to `existsSync` means the path will never match a real filesystem entry, so the condition on line 28 always fails, `""` gets cached permanently, and Windows users always fall back to `~/Downloads` — exactly the bug this PR intends to fix.
```suggestion
const rawPath = await runPowerShellScript(
`(Get-ItemProperty -Path "${WINDOWS_REG_DOWNLOAD_KEY}")."${WINDOWS_REG_DOWNLOAD_VALUE}"`,
{ timeout: 2000 },
).catch(() => null);
const path = rawPath?.trim() || null;
if (path && existsSync(path)) {
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/immich/src/utils/download.ts
Line: 18-36
Comment:
**Permanent cache prevents recovery from transient failures**
When the registry read fails (timeout, PowerShell unavailable, etc.) or the path doesn't exist, `""` is stored in the cache indefinitely. On subsequent calls `cache.has("download_folder")` is `true`, so the registry is never queried again and the user is silently stuck with `~/Downloads` for the lifetime of the cache. Similarly, if the user later moves their Downloads folder, the previously-cached registry path becomes stale and cannot self-correct.
Consider either not caching failure states (omit the `cache.set("download_folder", "")` line), or using a short TTL if the `Cache` API supports it.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "chores: add changelogs for v2" | Re-trigger Greptile |
| const path = await runPowerShellScript( | ||
| `(Get-ItemProperty -Path "${WINDOWS_REG_DOWNLOAD_KEY}")."${WINDOWS_REG_DOWNLOAD_VALUE}"`, | ||
| { timeout: 2000 }, | ||
| ).catch(() => null); | ||
|
|
||
| if (path && existsSync(path)) { | ||
| const realPath = await realpath(path); | ||
| cache.set("download_folder", realPath); | ||
| return realPath; | ||
| } |
There was a problem hiding this comment.
runPowerShellScript output not trimmed — existsSync will always return false
runPowerShellScript returns raw stdout, which includes a trailing newline (\r\n on Windows). Passing that untrimmed string to existsSync means the path will never match a real filesystem entry, so the condition on line 28 always fails, "" gets cached permanently, and Windows users always fall back to ~/Downloads — exactly the bug this PR intends to fix.
| const path = await runPowerShellScript( | |
| `(Get-ItemProperty -Path "${WINDOWS_REG_DOWNLOAD_KEY}")."${WINDOWS_REG_DOWNLOAD_VALUE}"`, | |
| { timeout: 2000 }, | |
| ).catch(() => null); | |
| if (path && existsSync(path)) { | |
| const realPath = await realpath(path); | |
| cache.set("download_folder", realPath); | |
| return realPath; | |
| } | |
| const rawPath = await runPowerShellScript( | |
| `(Get-ItemProperty -Path "${WINDOWS_REG_DOWNLOAD_KEY}")."${WINDOWS_REG_DOWNLOAD_VALUE}"`, | |
| { timeout: 2000 }, | |
| ).catch(() => null); | |
| const path = rawPath?.trim() || null; | |
| if (path && existsSync(path)) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/immich/src/utils/download.ts
Line: 23-32
Comment:
**`runPowerShellScript` output not trimmed — `existsSync` will always return false**
`runPowerShellScript` returns raw stdout, which includes a trailing newline (`\r\n` on Windows). Passing that untrimmed string to `existsSync` means the path will never match a real filesystem entry, so the condition on line 28 always fails, `""` gets cached permanently, and Windows users always fall back to `~/Downloads` — exactly the bug this PR intends to fix.
```suggestion
const rawPath = await runPowerShellScript(
`(Get-ItemProperty -Path "${WINDOWS_REG_DOWNLOAD_KEY}")."${WINDOWS_REG_DOWNLOAD_VALUE}"`,
{ timeout: 2000 },
).catch(() => null);
const path = rawPath?.trim() || null;
if (path && existsSync(path)) {
```
How can I resolve this? If you propose a fix, please make it concise.| async function windowsGetDownloadFolder() { | ||
| if (cache.has("download_folder")) { | ||
| return cache.get("download_folder") || null; | ||
| } | ||
|
|
||
| const path = await runPowerShellScript( | ||
| `(Get-ItemProperty -Path "${WINDOWS_REG_DOWNLOAD_KEY}")."${WINDOWS_REG_DOWNLOAD_VALUE}"`, | ||
| { timeout: 2000 }, | ||
| ).catch(() => null); | ||
|
|
||
| if (path && existsSync(path)) { | ||
| const realPath = await realpath(path); | ||
| cache.set("download_folder", realPath); | ||
| return realPath; | ||
| } | ||
|
|
||
| cache.set("download_folder", ""); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Permanent cache prevents recovery from transient failures
When the registry read fails (timeout, PowerShell unavailable, etc.) or the path doesn't exist, "" is stored in the cache indefinitely. On subsequent calls cache.has("download_folder") is true, so the registry is never queried again and the user is silently stuck with ~/Downloads for the lifetime of the cache. Similarly, if the user later moves their Downloads folder, the previously-cached registry path becomes stale and cannot self-correct.
Consider either not caching failure states (omit the cache.set("download_folder", "") line), or using a short TTL if the Cache API supports it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/immich/src/utils/download.ts
Line: 18-36
Comment:
**Permanent cache prevents recovery from transient failures**
When the registry read fails (timeout, PowerShell unavailable, etc.) or the path doesn't exist, `""` is stored in the cache indefinitely. On subsequent calls `cache.has("download_folder")` is `true`, so the registry is never queried again and the user is silently stuck with `~/Downloads` for the lifetime of the cache. Similarly, if the user later moves their Downloads folder, the previously-cached registry path becomes stale and cannot self-correct.
Consider either not caching failure states (omit the `cache.set("download_folder", "")` line), or using a short TTL if the `Cache` API supports it.
How can I resolve this? If you propose a fix, please make it concise.
Description
Previously, when downloading an asset on Windows, it always went to
C:\Users\<user>\Downloads. This caused problems for users that moved their download folder to another location (see #27396)Now, users can customize their download folder in the preferences.
Alternatively, if they don't specify a download folder and they're on Windows, the download folder path detection has been improved, looking for the value in the registry.
On MacOS, default path will still always be
~/DownloadsScreencast
Checklist
npm run buildand tested this distribution build in Raycastassetsfolder are used by the extension itselfREADMEare located outside the metadata folder if they were not generated with our metadata tool