Add XDG user directories mapping to Windows Library folders#459
Add XDG user directories mapping to Windows Library folders#459Copilot wants to merge 7 commits intodevelopmentfrom
Conversation
Co-authored-by: crramirez <8397379+crramirez@users.noreply.github.com>
Co-authored-by: crramirez <8397379+crramirez@users.noreply.github.com>
…var, fix uninstall Co-authored-by: crramirez <8397379+crramirez@users.noreply.github.com>
Co-authored-by: crramirez <8397379+crramirez@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new Settings option to pengwin-setup to map Linux XDG user directories (Desktop, Documents, Downloads, etc.) onto the corresponding Windows Library folders (via symlinks), improving WSL/GUI app integration with Windows file locations.
Changes:
- Introduces an installer script to install
xdg-user-dirs, generateuser-dirs.dirs, resolve Windows Library locations, and create symlinks. - Adds an uninstaller to remove only the created symlinks, with optional removal of the
xdg-user-dirspackage. - Wires the feature into Settings/Uninstall menus, bash completion, and CI test execution.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
pengwin-setup.d/xdg-user-dirs.sh |
New installer/mapping logic for XDG dirs → Windows folders (wslvar/wslpath, symlink creation). |
pengwin-setup.d/uninstall/xdg-user-dirs.sh |
New uninstaller to remove symlinks and optionally uninstall xdg-user-dirs. |
pengwin-setup.d/settings.sh |
Adds USERDIRS entry to Settings menu and dispatch to the installer script. |
pengwin-setup.d/uninstall.sh |
Adds USERDIRS entry to Uninstall menu and dispatch to the new uninstaller script. |
completions/pengwin-setup |
Adds USERDIRS to SETTINGS and UNINSTALL bash completion options. |
tests/xdg-user-dirs.sh |
Adds a basic install/uninstall shunit2 test for the new feature. |
tests/run_tests.sh |
Includes the new test in the test runner. |
| if [[ -n "${target_path}" ]]; then | ||
| create_xdg_symlink "${xdg_dir_path}" "${target_path}" | ||
| else | ||
| echo "Could not find Windows path for ${xdg_const}" | ||
| fi | ||
| done | ||
|
|
||
| echo "XDG user directories mapped successfully!" | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
map_xdg_dirs() always prints a success message and returns 0 even if no directories were actually mapped (e.g., wslvar/wslpath unavailable, or create_xdg_symlink() fails because the target path is missing/non-empty). Consider tracking per-directory failures/successes and returning non-zero (and/or a more accurate summary) when nothing is mapped or any mapping fails.
| elif [[ -d "${xdg_path}" ]]; then | ||
| if [[ -z "$(ls -A "${xdg_path}" 2>/dev/null)" ]]; then | ||
| echo "Removing empty directory: ${xdg_path}" | ||
| rmdir "${xdg_path}" | ||
| else | ||
| echo "Directory not empty, cannot replace: ${xdg_path}" | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
The empty-directory check uses ls -A and then rmdir, which introduces a TOCTOU race and unnecessary process spawning. Since rmdir already fails on non-empty directories, prefer attempting rmdir directly and checking its exit status (and only erroring if it fails for non-empty).
| while IFS= read -r line; do | ||
| # Parse "Key : Value" format | ||
| key=$(echo "${line}" | cut -d':' -f1 | sed 's/[[:space:]]*$//') | ||
| value=$(echo "${line}" | cut -d':' -f2- | sed 's/^[[:space:]]*//' | tr -d '\r') | ||
| if [[ -n "${key}" && -n "${value}" ]]; then | ||
| WSLVAR_L_CACHE["${key}"]="${value}" | ||
| fi | ||
| done < <(wslvar -L 2>/dev/null) |
There was a problem hiding this comment.
load_wslvar_*_cache() assumes every output line contains a ':' separator. If wslvar outputs an error/placeholder line (or localized output), the current parsing can create bogus cache entries. Consider skipping lines that don't match the expected key : value format before populating the cache.
| # Replace $HOME with actual home directory | ||
| dir_value="${dir_value/\$HOME/${HOME}}" | ||
|
|
||
| echo "${dir_value}" |
There was a problem hiding this comment.
The $HOME substitution only replaces the first occurrence (${dir_value/\$HOME/...}); if the value contains $HOME more than once, the path will be partially expanded. Consider using the global replacement form and (optionally) handling ${HOME} too for robustness.
| # Replace $HOME with actual home directory | ||
| dir_value="${dir_value/\$HOME/${HOME}}" | ||
|
|
There was a problem hiding this comment.
The $HOME substitution only replaces the first occurrence (${dir_value/\$HOME/...}); if the value contains $HOME more than once, the path will be partially expanded. Consider using the global replacement form and (optionally) handling ${HOME} too for robustness.
tests/xdg-user-dirs.sh
Outdated
| # After uninstall, user-dirs.dirs should be deleted and regenerated | ||
| # by xdg-user-dirs-update | ||
| run test -f "$HOME/.config/user-dirs.dirs" | ||
| assertFalse "user-dirs.dirs file should be deleted after uninstall" "$?" |
There was a problem hiding this comment.
The comment says user-dirs.dirs is "deleted and regenerated", but the test asserts the file is absent after uninstall. Please update the comment to match the actual expected behavior (or adjust the assertion if regeneration is intended).
There was a problem hiding this comment.
Fixed in c9f7b45. Updated the comment to accurately describe that when the package is removed, the user-dirs.dirs file is deleted (not regenerated).
Co-authored-by: crramirez <8397379+crramirez@users.noreply.github.com>
pengwin-setup.d/xdg-user-dirs.shthat:xdg-user-dirspackagexdg-user-dirs-updateto generate XDG dirs configwslvar -Landwslvar -Swslpath -u$XDG_CONFIG_HOME/user-dirs.dirsto support localizationpengwin-setup.d/settings.shmenupengwin-setup.d/uninstall/xdg-user-dirs.shxdg-user-dirs-updateto regenerate folders only when keeping the packagepengwin-setup.d/uninstall.shmenucompletions/pengwin-setupwith new optiontests/xdg-user-dirs.shtests/run_tests.shOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.