[Dribbblish] Added easy auto install for linux users#1249
[Dribbblish] Added easy auto install for linux users#1249cameraman-pritam wants to merge 2 commits into
Conversation
WalkthroughDocumentation for the Dribbblish Spicetify theme was comprehensively restructured with improved section organization and formatting. Two new Linux installation and uninstallation automation scripts were added to streamline theme setup and removal. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@Dribbblish/install-linux.sh`:
- Around line 14-24: The curl invocation that creates ZIP_FILE should include
the -f flag to fail on HTTP errors and avoid producing an invalid archive; also
ensure temporary files are removed on script exit by adding a trap that deletes
ZIP_FILE and EXTRACT_PATH (and any extracted temp dir) in case of success or
failure, and call that cleanup before exit paths that follow the unzip step;
locate the ZIP_FILE and EXTRACT_PATH variables and the curl/unzip commands in
the script to add the -f flag and the trap cleanup logic tied to those symbols.
- Around line 51-54: The heredoc that appends to CONFIG_FILE uses backslashes
(e.g., \w and \156) which may be interpreted by the shell; change the heredoc to
a quoted form to prevent expansion by replacing <<EOF with <<'EOF' (or
alternatively use printf '%s\n' to write the literal lines) so the literal
strings "xpui.js_find_8008=,(\\w+=)32," and "xpui.js_repl_8008=,\\156," are
written exactly as intended.
In `@Dribbblish/README.md`:
- Around line 104-105: The README uses inconsistent branch names for the Windows
scripts: update the install URL in the Dribbblish README (the Invoke-WebRequest
line that points to
https://raw.githubusercontent.com/spicetify/spicetify-themes/master/Dribbblish/install.ps1)
so it matches the uninstall URL branch (or vice versa) — ensure both the install
and uninstall Invoke-WebRequest links reference the same branch (e.g., both use
v2 or both use master) to avoid mismatches.
- Around line 113-115: Update the fenced code blocks in README.md that currently
contain bare filenames (e.g., the blocks showing "install-linux.sh" and
"uninstall-linux.sh") to include a language specifier like "text" or "bash" (for
example change ``` to ```text or ```bash); apply the same change to the other
similar blocks noted (lines ~196-198) so all filename-only code fences declare a
language.
- Line 218: Remove the dangling code-block marker from the README: locate the
orphaned triple-backtick (```) at the end of Dribbblish/README.md and delete it
so there is no unclosed code fence left in the file.
In `@Dribbblish/uninstall-linux.sh`:
- Around line 1-4: Add a strict-fail at the top of uninstall-linux.sh by
enabling "set -e" and mirror the install script's safety check by verifying
spicetify is installed before running any commands; if the "spicetify" binary is
missing, print a clear error and exit non-zero. Specifically, update
uninstall-linux.sh to add "set -e" near the shebang and add a check (e.g., using
command existence test for "spicetify") that aborts with an explanatory message
if not found, then proceed to call "spicetify config current_theme \"\"" only
when the check passes.
🧹 Nitpick comments (2)
Dribbblish/install-linux.sh (1)
38-39: Standalonespicetifycall has no clear purpose.Line 38 calls
spicetifywithout arguments. This typically prints help/version info but doesn't configure anything. If the intent is to initialize config, considerspicetify configor remove the line if unnecessary.Proposed fix
echo "Configuring..." -spicetify spicetify config inject_css 1 replace_colors 1 overwrite_assets 1 inject_theme_js 1 current_theme DribbblishDribbblish/uninstall-linux.sh (1)
6-19: Consider removing the Dribbblish theme directory during uninstall.The script clears the config and removes patch lines but doesn't remove the
Themes/Dribbblishdirectory that was copied during installation. This leaves orphaned files on the system.Proposed enhancement
# Get Spicetify config path CONFIG_PATH=$(spicetify -c) +SPICE_PATH="$(dirname "$CONFIG_PATH")" +THEME_PATH="$SPICE_PATH/Themes/Dribbblish" # Create a backup for safety cp "$CONFIG_PATH" "$CONFIG_PATH.bak" +# Remove theme directory +if [ -d "$THEME_PATH" ]; then + rm -rf "$THEME_PATH" + echo "Removed theme directory: $THEME_PATH" +fi + # Remove xpui.js_find_8008 line
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Dribbblish/README.mdDribbblish/install-linux.shDribbblish/uninstall-linux.sh
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
Dribbblish/README.md
113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
196-196: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
218-218: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (4)
Dribbblish/install-linux.sh (2)
1-10: Good prerequisite check for spicetify.The shebang,
set -efor fail-fast behavior, and the spicetify existence check are well implemented.
57-62: Version check logic for backup apply is reasonable.The conditional to run
spicetify backup applywhen no version line exists (first-time setup) vsspicetify applyotherwise is a sensible approach.Dribbblish/uninstall-linux.sh (1)
12-16:sed -iwithout suffix is not portable to macOS.The
-iflag behavior differs between GNU sed (Linux) and BSD sed (macOS). On macOS,sed -irequires an extension argument. Since the script is namedinstall-linux.sh, this may be intentional, but worth noting if cross-platform support is desired.If macOS support is intended in the future, consider:
-sed -i '/xpui.js_find_8008/d' "$CONFIG_PATH" -sed -i '/xpui.js_repl_8008/d' "$CONFIG_PATH" +sed -i.tmp '/xpui.js_find_8008/d' "$CONFIG_PATH" && rm -f "$CONFIG_PATH.tmp" +sed -i.tmp '/xpui.js_repl_8008/d' "$CONFIG_PATH" && rm -f "$CONFIG_PATH.tmp"Dribbblish/README.md (1)
1-56: Well-structured documentation with clear previews.The reorganized preview section with all color schemes is comprehensive and visually helpful for users evaluating the theme.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| ZIP_FILE="/tmp/spicetify-themes.zip" | ||
| EXTRACT_PATH="/tmp/spicetify-themes-master" | ||
|
|
||
| curl -L \ | ||
| "https://github.com/spicetify/spicetify-themes/archive/refs/heads/master.zip" \ | ||
| -o "$ZIP_FILE" | ||
|
|
||
| echo "Extracting themes package..." | ||
|
|
||
| rm -rf "$EXTRACT_PATH" | ||
| unzip -q "$ZIP_FILE" -d /tmp |
There was a problem hiding this comment.
Add -f flag to curl and cleanup temp files on exit.
The curl command should use -f (fail on HTTP errors) to avoid silently creating an empty/invalid zip on 404s. Additionally, the temporary files are never cleaned up after use.
Proposed fix
+# Cleanup on exit
+cleanup() {
+ rm -rf "$ZIP_FILE" "$EXTRACT_PATH"
+}
+trap cleanup EXIT
+
ZIP_FILE="/tmp/spicetify-themes.zip"
EXTRACT_PATH="/tmp/spicetify-themes-master"
-curl -L \
+curl -fL \
"https://github.com/spicetify/spicetify-themes/archive/refs/heads/master.zip" \
-o "$ZIP_FILE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ZIP_FILE="/tmp/spicetify-themes.zip" | |
| EXTRACT_PATH="/tmp/spicetify-themes-master" | |
| curl -L \ | |
| "https://github.com/spicetify/spicetify-themes/archive/refs/heads/master.zip" \ | |
| -o "$ZIP_FILE" | |
| echo "Extracting themes package..." | |
| rm -rf "$EXTRACT_PATH" | |
| unzip -q "$ZIP_FILE" -d /tmp | |
| # Cleanup on exit | |
| cleanup() { | |
| rm -rf "$ZIP_FILE" "$EXTRACT_PATH" | |
| } | |
| trap cleanup EXIT | |
| ZIP_FILE="/tmp/spicetify-themes.zip" | |
| EXTRACT_PATH="/tmp/spicetify-themes-master" | |
| curl -fL \ | |
| "https://github.com/spicetify/spicetify-themes/archive/refs/heads/master.zip" \ | |
| -o "$ZIP_FILE" | |
| echo "Extracting themes package..." | |
| rm -rf "$EXTRACT_PATH" | |
| unzip -q "$ZIP_FILE" -d /tmp |
🤖 Prompt for AI Agents
In `@Dribbblish/install-linux.sh` around lines 14 - 24, The curl invocation that
creates ZIP_FILE should include the -f flag to fail on HTTP errors and avoid
producing an invalid archive; also ensure temporary files are removed on script
exit by adding a trap that deletes ZIP_FILE and EXTRACT_PATH (and any extracted
temp dir) in case of success or failure, and call that cleanup before exit paths
that follow the unzip step; locate the ZIP_FILE and EXTRACT_PATH variables and
the curl/unzip commands in the script to add the -f flag and the trap cleanup
logic tied to those symbols.
| cat >> "$CONFIG_FILE" <<EOF | ||
| xpui.js_find_8008=,(\\w+=)32, | ||
| xpui.js_repl_8008=,\\156, | ||
| EOF |
There was a problem hiding this comment.
Escape sequences may behave unexpectedly in heredoc.
The \w and \156 in the heredoc might be interpreted by bash depending on context. Consider using a quoted heredoc (<<'EOF') to prevent any shell interpretation of backslashes.
Proposed fix
- cat >> "$CONFIG_FILE" <<EOF
+ cat >> "$CONFIG_FILE" <<'EOF'
xpui.js_find_8008=,(\w+=)32,
xpui.js_repl_8008=,\156,
EOF🤖 Prompt for AI Agents
In `@Dribbblish/install-linux.sh` around lines 51 - 54, The heredoc that appends
to CONFIG_FILE uses backslashes (e.g., \w and \156) which may be interpreted by
the shell; change the heredoc to a quoted form to prevent expansion by replacing
<<EOF with <<'EOF' (or alternatively use printf '%s\n' to write the literal
lines) so the literal strings "xpui.js_find_8008=,(\\w+=)32," and
"xpui.js_repl_8008=,\\156," are written exactly as intended.
| Invoke-WebRequest -UseBasicParsing "https://raw.githubusercontent.com/spicetify/spicetify-themes/master/Dribbblish/install.ps1" | Invoke-Expression | ||
| ```` |
There was a problem hiding this comment.
Inconsistent branch references between install and uninstall URLs.
The Windows auto-install script references master branch while the uninstall script references v2 branch. This may cause version mismatches or broken functionality if the branches diverge.
# Install (line 104) - uses master:
https://raw.githubusercontent.com/spicetify/spicetify-themes/master/Dribbblish/install.ps1
# Uninstall (line 187) - uses v2:
https://raw.githubusercontent.com/spicetify/spicetify-themes/v2/Dribbblish/uninstall.ps1Consider aligning both to the same branch for consistency.
Also applies to: 187-188
🤖 Prompt for AI Agents
In `@Dribbblish/README.md` around lines 104 - 105, The README uses inconsistent
branch names for the Windows scripts: update the install URL in the Dribbblish
README (the Invoke-WebRequest line that points to
https://raw.githubusercontent.com/spicetify/spicetify-themes/master/Dribbblish/install.ps1)
so it matches the uninstall URL branch (or vice versa) — ensure both the install
and uninstall Invoke-WebRequest links reference the same branch (e.g., both use
v2 or both use master) to avoid mismatches.
| ``` | ||
| install-linux.sh | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to code blocks.
Per static analysis, these code blocks should have a language specified. Since they contain filenames, consider using text or bash as the language.
Proposed fix
-```
+```text
install-linux.sh
```diff
-```
+```text
uninstall-linux.sh
</details>
Also applies to: 196-198
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @Dribbblish/README.md around lines 113 - 115, Update the fenced code blocks
in README.md that currently contain bare filenames (e.g., the blocks showing
"install-linux.sh" and "uninstall-linux.sh") to include a language specifier
like "text" or "bash" (for example change totext or ```bash); apply the
same change to the other similar blocks noted (lines ~196-198) so all
filename-only code fences declare a language.
</details>
<!-- fingerprinting:phantom:poseidon:ocelot -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| --- | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Remove orphaned code block marker.
There's an unclosed/orphaned code block at the end of the file that serves no purpose.
Proposed fix
---
-
-```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
218-218: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@Dribbblish/README.md` at line 218, Remove the dangling code-block marker from
the README: locate the orphaned triple-backtick (```) at the end of
Dribbblish/README.md and delete it so there is no unclosed code fence left in
the file.
| #!/usr/bin/env bash | ||
|
|
||
| # Clear current theme | ||
| spicetify config current_theme "" |
There was a problem hiding this comment.
Add set -e and spicetify existence check for consistency.
Unlike the install script, this script lacks set -e and doesn't verify spicetify is installed before attempting to use it. This could lead to confusing errors.
Proposed fix
#!/usr/bin/env bash
+set -e
+
+# Check for spicetify
+if ! command -v spicetify >/dev/null 2>&1; then
+ echo "Spicetify not found"
+ exit 1
+fi
+
# Clear current theme
spicetify config current_theme ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/usr/bin/env bash | |
| # Clear current theme | |
| spicetify config current_theme "" | |
| #!/usr/bin/env bash | |
| set -e | |
| # Check for spicetify | |
| if ! command -v spicetify >/dev/null 2>&1; then | |
| echo "Spicetify not found" | |
| exit 1 | |
| fi | |
| # Clear current theme | |
| spicetify config current_theme "" |
🤖 Prompt for AI Agents
In `@Dribbblish/uninstall-linux.sh` around lines 1 - 4, Add a strict-fail at the
top of uninstall-linux.sh by enabling "set -e" and mirror the install script's
safety check by verifying spicetify is installed before running any commands; if
the "spicetify" binary is missing, print a clear error and exit non-zero.
Specifically, update uninstall-linux.sh to add "set -e" near the shebang and add
a check (e.g., using command existence test for "spicetify") that aborts with an
explanatory message if not found, then proceed to call "spicetify config
current_theme \"\"" only when the check passes.
1 participant
added files install-linux.sh and uninstall-linux.sh
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.