Skip to content

Conversation

@Tearran
Copy link
Owner

@Tearran Tearran commented Aug 9, 2025

Summary by CodeRabbit

  • New Features
    • Added a script to automatically generate PNG icon sets in multiple sizes from SVG files, streamlining the icon creation process.
    • The script provides clear prompts and feedback, including checks for required tools and source files.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 9, 2025

Walkthrough

A new Bash script, 60_icon_set_from_svg.sh, has been added to automate the conversion of SVG icon files into PNG format at multiple standard icon sizes. The script checks for dependencies, validates source directories, and processes SVG files to generate PNGs, placing them in the appropriate directories if they do not already exist or are outdated.

Changes

Cohort / File(s) Change Summary
SVG to PNG Icon Conversion Script
SDK/60_icon_set_from_svg.sh
Introduces a Bash script that automates batch conversion of SVG icons to PNGs at multiple sizes, with dependency and directory checks.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Script
    participant System

    User->>Script: Run 60_icon_set_from_svg.sh
    Script->>System: Check for 'convert' command
    alt 'convert' missing
        Script->>User: Prompt to install ImageMagick
        User->>Script: Responds (yes/no)
        alt User agrees
            Script->>System: Install ImageMagick
        else User declines
            Script->>User: Exit with error
        end
    end
    Script->>System: Check SVG source directory and files
    alt SVGs found
        loop For each SVG file
            loop For each target size
                Script->>System: Check if PNG exists or is outdated
                alt Needs conversion
                    Script->>System: Convert SVG to PNG
                    Script->>User: Print success/failure message
                end
            end
        end
    else
        Script->>User: Exit with error (no SVGs)
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

Poem

A script has hopped into the scene,
Turning SVGs to PNGs—so clean!
With checks for tools and folders neat,
It makes your icon set complete.
From scalable art to pixel delight,
The bunnies code through day and night!
🐇✨

Note

🔌 MCP (Model Context Protocol) integration is now available in Early Access!

Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch icons-script

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Tearran Tearran marked this pull request as ready for review August 9, 2025 19:58
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
share/icons/icon_set_from_svg.sh (2)

48-56: Optional: make sizes configurable via CLI.

Allow overriding SIZES by passing arguments: ./icon_set_from_svg.sh 16 32 48 128.

-# List of desired sizes
-SIZES=(16 32 64 128 256 512)
+# List of desired sizes (can be overridden by CLI)
+SIZES=("${@:-16 32 64 128 256 512}")

1-63: Future alignment with your scaffolding/metadata standard.

If you want this to integrate with your tools’ metadata-driven help/registry, we can add a .conf and an about* function (or a thin wrapper in tools/) without blocking this PR. I can open a follow-up issue and prepare the scaffolding.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f014171 and 2e41d22.

⛔ Files ignored due to path filters (43)
  • share/icons/hicolor/128x128/armbian-tux_v1.5.png is excluded by !**/*.png
  • share/icons/hicolor/128x128/armbian_cpu_v2.png is excluded by !**/*.png
  • share/icons/hicolor/128x128/armbian_font.png is excluded by !**/*.png
  • share/icons/hicolor/128x128/configng-tux_v1.png is excluded by !**/*.png
  • share/icons/hicolor/128x128/configng-tux_v2.png is excluded by !**/*.png
  • share/icons/hicolor/128x128/configng_sprocket_v2.png is excluded by !**/*.png
  • share/icons/hicolor/128x128/configng_sproket_v1.png is excluded by !**/*.png
  • share/icons/hicolor/16x16/armbian-tux_v1.5.png is excluded by !**/*.png
  • share/icons/hicolor/16x16/armbian_cpu_v2.png is excluded by !**/*.png
  • share/icons/hicolor/16x16/armbian_font.png is excluded by !**/*.png
  • share/icons/hicolor/16x16/configng-tux_v1.png is excluded by !**/*.png
  • share/icons/hicolor/16x16/configng-tux_v2.png is excluded by !**/*.png
  • share/icons/hicolor/16x16/configng_sprocket_v2.png is excluded by !**/*.png
  • share/icons/hicolor/16x16/configng_sproket_v1.png is excluded by !**/*.png
  • share/icons/hicolor/256x256/armbian-tux_v1.5.png is excluded by !**/*.png
  • share/icons/hicolor/256x256/armbian_cpu_v2.png is excluded by !**/*.png
  • share/icons/hicolor/256x256/armbian_font.png is excluded by !**/*.png
  • share/icons/hicolor/256x256/configng-tux_v1.png is excluded by !**/*.png
  • share/icons/hicolor/256x256/configng-tux_v2.png is excluded by !**/*.png
  • share/icons/hicolor/256x256/configng_sprocket_v2.png is excluded by !**/*.png
  • share/icons/hicolor/256x256/configng_sproket_v1.png is excluded by !**/*.png
  • share/icons/hicolor/32x32/armbian-tux_v1.5.png is excluded by !**/*.png
  • share/icons/hicolor/32x32/armbian_cpu_v2.png is excluded by !**/*.png
  • share/icons/hicolor/32x32/armbian_font.png is excluded by !**/*.png
  • share/icons/hicolor/32x32/configng-tux_v1.png is excluded by !**/*.png
  • share/icons/hicolor/32x32/configng-tux_v2.png is excluded by !**/*.png
  • share/icons/hicolor/32x32/configng_sprocket_v2.png is excluded by !**/*.png
  • share/icons/hicolor/32x32/configng_sproket_v1.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/armbian-tux_v1.5.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/armbian_cpu_v2.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/armbian_font.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/armbian_tux_v1.5.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng-tux_v1.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng-tux_v2.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng_sprocket_v2.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng_sproket_v1.png is excluded by !**/*.png
  • share/icons/hicolor/64x64/armbian-tux_v1.5.png is excluded by !**/*.png
  • share/icons/hicolor/64x64/armbian_cpu_v2.png is excluded by !**/*.png
  • share/icons/hicolor/64x64/armbian_font.png is excluded by !**/*.png
  • share/icons/hicolor/64x64/configng-tux_v1.png is excluded by !**/*.png
  • share/icons/hicolor/64x64/configng-tux_v2.png is excluded by !**/*.png
  • share/icons/hicolor/64x64/configng_sprocket_v2.png is excluded by !**/*.png
  • share/icons/hicolor/64x64/configng_sproket_v1.png is excluded by !**/*.png
📒 Files selected for processing (1)
  • share/icons/icon_set_from_svg.sh (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Tearran
PR: Tearran/configng-v2#36
File: src/network/interface/network_manager.sh:46-53
Timestamp: 2025-06-23T18:54:21.952Z
Learning: Tearran prefers heredoc over echo/printf for multi-line templating work, especially for generating dynamic content from metadata. Heredoc provides better readability, easier maintenance, and is more forgiving for complex formatting than chained echo statements.
Learnt from: Tearran
PR: Tearran/configng-v2#71
File: tools/00_start_here.sh:243-246
Timestamp: 2025-07-15T07:35:26.542Z
Learning: Tearran prefers applying the scaffolding standard systematically across tools scripts, including .conf metadata files and function naming conventions, to create project-wide consistency between tools and modules.
Learnt from: Tearran
PR: Tearran/configng-v2#36
File: src/network/interface/network_manager.sh:46-53
Timestamp: 2025-06-23T18:48:41.687Z
Learning: Tearran prefers metadata-driven help generation using .conf files that can be parsed with basic bash tools (grep, sed, cat, jq). The .conf format contains rich metadata including feature names, descriptions, options, and requirements that should drive dynamic _about_ function generation rather than hardcoded help text.
Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T17:36:25.934Z
Learning: Tearran needs a registry system to track multiple helper functions (_*_*) beyond the main function and _about_* pattern for parsing and metadata purposes.
Learnt from: Tearran
PR: Tearran/configng-v2#0
File: :0-0
Timestamp: 2025-07-18T18:14:15.440Z
Learning: When Tearran suggests creating an issue for documentation updates instead of implementing them directly, he prefers this systematic approach for substantial changes that need focused planning and discussion, especially when it separates immediate PR concerns from broader improvements.
Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T15:56:11.653Z
Learning: Tearran is concerned about preventing false positive test patterns project-wide and wants systematic prevention rather than case-by-case fixes.
Learnt from: Tearran
PR: Tearran/configng-v2#52
File: tools/25_docstring.sh:46-56
Timestamp: 2025-06-29T19:11:03.193Z
Learning: When Tearran requests creating an issue for future enhancements that need separate focus, he prefers to acknowledge that the current code works as expected while planning the enhancement work separately rather than blocking current functionality.
Learnt from: Tearran
PR: Tearran/configng-v2#52
File: tools/25_docstring.sh:46-56
Timestamp: 2025-06-29T19:11:03.193Z
Learning: When Tearran requests creating an issue for future enhancements that need separate focus, he prefers to acknowledge that the current code works as expected while planning the enhancement work separately rather than blocking current functionality.
Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T21:50:00.575Z
Learning: Tearran's project currently uses only a 'contributor' field in configuration files, not a 'maintained' field. The contributor field appears to indicate both authorship and maintenance responsibility.
Learnt from: Tearran
PR: Tearran/configng-v2#43
File: tools/configng_v2.sh:14-17
Timestamp: 2025-06-25T03:11:52.041Z
Learning: When Tearran states their code works as expected, accept that at face value rather than assuming it needs fixes or has missing functionality.
📚 Learning: 2025-07-18T17:27:45.451Z
Learnt from: Tearran
PR: Tearran/configng-v2#84
File: docs/cockpit.md:7-8
Timestamp: 2025-07-18T17:27:45.451Z
Learning: The workflow/30_docstring.sh script correctly handles image copying during documentation generation. It finds images in module source directories (like src/software/user/cockpit.png), copies them to docs/, and generates correct relative markdown paths (./image.png). No manual intervention needed for image paths in auto-generated documentation.

Applied to files:

  • share/icons/icon_set_from_svg.sh
🪛 GitHub Actions: 01 Coding style check
share/icons/icon_set_from_svg.sh

[error] 10-25: editorconfig-checker: Wrong indentation type (spaces instead of tabs)


[error] 30-31: editorconfig-checker: Wrong indentation type (spaces instead of tabs)


[error] 38-39: editorconfig-checker: Wrong indentation type (spaces instead of tabs)


[error] 45-61: editorconfig-checker: Wrong indentation type (spaces instead of tabs)

🔇 Additional comments (1)
share/icons/icon_set_from_svg.sh (1)

10-25: Indentation issues resolved in share/icons/icon_set_from_svg.sh
Verified via grep: no lines begin with spaces. Tabs are now used for all indentation.

Comment on lines 1 to 6
#!/bin/bash

# Directory containing SVGs
SRC_DIR="share/icons/hicolor/scalable"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add strict mode and make paths robust/portable (use SCRIPT_DIR).

Enable safer bash defaults and resolve SRC_DIR relative to the script so it works regardless of the current working directory.

 #!/bin/bash
+
+set -euo pipefail
+IFS=$'\n\t'
 
 # Directory containing SVGs
-SRC_DIR="share/icons/hicolor/scalable"
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
+SRC_DIR="${SRC_DIR:-"$SCRIPT_DIR/hicolor/scalable"}"
 # List of desired sizes
 SIZES=(16 32 64 128 256 512)
📝 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.

Suggested change
#!/bin/bash
# Directory containing SVGs
SRC_DIR="share/icons/hicolor/scalable"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
#!/bin/bash
set -euo pipefail
IFS=$'\n\t'
# Directory containing SVGs
SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
SRC_DIR="${SRC_DIR:-"$SCRIPT_DIR/hicolor/scalable"}"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
🤖 Prompt for AI Agents
In share/icons/icon_set_from_svg.sh at lines 1 to 6, add bash strict mode by
including 'set -euo pipefail' at the top to enable safer script execution.
Define SCRIPT_DIR using a robust method to get the script's directory (e.g.,
using dirname and BASH_SOURCE) and update SRC_DIR to be relative to SCRIPT_DIR
instead of a fixed path, ensuring the script works correctly regardless of the
current working directory.

Comment on lines 9 to 26
if ! command -v convert &> /dev/null; then
echo "Error: ImageMagick 'convert' command not found."
read -p "Would you like to install ImageMagick using 'sudo apt install imagemagick'? [Y/n] " yn
case "$yn" in
[Yy]* | "" )
echo "Installing ImageMagick..."
sudo apt update && sudo apt install imagemagick
if ! command -v convert &> /dev/null; then
echo "Installation failed or 'convert' still not found. Exiting."
exit 1
fi
;;
* )
echo "Cannot proceed without ImageMagick. Exiting."
exit 1
;;
esac
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid interactive package installation; support ImageMagick 7 (“magick”) and fail fast.

Installing packages inside scripts is brittle and CI-hostile. Detect “magick” (IM7) or “convert” (IM6); otherwise exit with a clear message.

-# Check for ImageMagick's convert command
-if ! command -v convert &> /dev/null; then
-  echo "Error: ImageMagick 'convert' command not found."
-  read -p "Would you like to install ImageMagick using 'sudo apt install imagemagick'? [Y/n] " yn
-  case "$yn" in
-    [Yy]* | "" )
-      echo "Installing ImageMagick..."
-      sudo apt update && sudo apt install imagemagick
-      if ! command -v convert &> /dev/null; then
-        echo "Installation failed or 'convert' still not found. Exiting."
-        exit 1
-      fi
-      ;;
-    * )
-      echo "Cannot proceed without ImageMagick. Exiting."
-      exit 1
-      ;;
-  esac
-fi
+# Check for ImageMagick command (prefer IM7 `magick`, fallback to IM6 `convert`)
+IM_CMD=""
+if command -v magick >/dev/null 2>&1; then
+  IM_CMD="magick"
+elif command -v convert >/dev/null 2>&1; then
+  IM_CMD="convert"
+else
+  echo "Error: ImageMagick not found. Please install it (e.g., 'apt install imagemagick' or 'brew install imagemagick') and re-run."
+  exit 1
+fi
🧰 Tools
🪛 GitHub Actions: 01 Coding style check

[error] 10-25: editorconfig-checker: Wrong indentation type (spaces instead of tabs)

🤖 Prompt for AI Agents
In share/icons/icon_set_from_svg.sh around lines 9 to 26, remove the interactive
prompt and package installation commands. Instead, check if either the "magick"
command (ImageMagick 7) or the "convert" command (ImageMagick 6) is available.
If neither is found, immediately exit with a clear error message indicating that
ImageMagick is required. This avoids interactive installs and supports both IM6
and IM7.

Comment on lines 34 to 41
# Check if SVGs exist in the source directory
shopt -s nullglob
svg_files=("$SRC_DIR"/*.svg)
if [ ${#svg_files[@]} -eq 0 ]; then
echo "Error: No SVG files found in '$SRC_DIR'."
exit 1
fi
shopt -u nullglob
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Globbing only top-level SVGs misses category subfolders; include nested paths and keep nullglob safety.

Icon themes usually store scalable//*.svg. Cover both direct and one-level nested SVGs.

 # Check if SVGs exist in the source directory
 shopt -s nullglob
-svg_files=("$SRC_DIR"/*.svg)
+svg_files=("$SRC_DIR"/*.svg "$SRC_DIR"/*/*.svg)
 if [ ${#svg_files[@]} -eq 0 ]; then
   echo "Error: No SVG files found in '$SRC_DIR'."
   exit 1
 fi
 shopt -u nullglob
🧰 Tools
🪛 GitHub Actions: 01 Coding style check

[error] 38-39: editorconfig-checker: Wrong indentation type (spaces instead of tabs)

🤖 Prompt for AI Agents
In share/icons/icon_set_from_svg.sh around lines 34 to 41, the current glob
pattern only matches SVG files directly under the source directory, missing SVGs
in category subfolders. Update the glob pattern to include SVG files both at the
top level and one directory deep (e.g., scalable/*.svg and scalable/*/*.svg)
while keeping nullglob enabled to avoid errors when no files match.

Comment on lines 43 to 62
# Loop over each SVG file in the scalable directory
for svg in "${svg_files[@]}"; do
# Extract the base filename without extension
base=$(basename "$svg" .svg)
# For each size, generate the PNG in the corresponding directory
for size in "${SIZES[@]}"; do
OUT_DIR="share/icons/hicolor/${size}x${size}"
mkdir -p "$OUT_DIR"
OUT_FILE="${OUT_DIR}/${base}.png"
# Only generate if missing or source SVG is newer
if [[ ! -f "$OUT_FILE" || "$svg" -nt "$OUT_FILE" ]]; then
convert -background none -resize ${size}x${size} "$svg" "$OUT_FILE"
if [ $? -eq 0 ]; then
echo "Generated $OUT_FILE"
else
echo "Failed to convert $svg to $OUT_FILE"
fi
fi
done
done
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Preserve icon theme category structure (apps, actions, etc.) in output directories per Freedesktop Icon Theme spec.

Currently outputs to hicolor/x only, flattening categories. Mirror the category from the scalable tree so consumers find icons under hicolor/x/.

 # Loop over each SVG file in the scalable directory
 for svg in "${svg_files[@]}"; do
   # Extract the base filename without extension
   base=$(basename "$svg" .svg)
+  # Determine category (e.g., apps, actions) from parent directory when present
+  parent_dir="$(basename "$(dirname "$svg")")"
+  category="$parent_dir"
+  # If SVGs are directly under 'scalable', default to 'apps' (common convention)
+  if [[ "$category" == "scalable" ]]; then
+    category="apps"
+  fi
   # For each size, generate the PNG in the corresponding directory
   for size in "${SIZES[@]}"; do
-    OUT_DIR="share/icons/hicolor/${size}x${size}"
+    OUT_DIR="$SCRIPT_DIR/hicolor/${size}x${size}/${category}"
     mkdir -p "$OUT_DIR"
     OUT_FILE="${OUT_DIR}/${base}.png"
     # Only generate if missing or source SVG is newer
     if [[ ! -f "$OUT_FILE" || "$svg" -nt "$OUT_FILE" ]]; then
-      convert -background none -resize ${size}x${size} "$svg" "$OUT_FILE"
+      "$IM_CMD" -background none -density 384 \
+        -resize "${size}x${size}" -gravity center -extent "${size}x${size}" \
+        -strip "$svg" "$OUT_FILE"
       if [ $? -eq 0 ]; then
         echo "Generated $OUT_FILE"
       else
         echo "Failed to convert $svg to $OUT_FILE"
       fi
     fi
   done
 done
🧰 Tools
🪛 GitHub Actions: 01 Coding style check

[error] 45-61: editorconfig-checker: Wrong indentation type (spaces instead of tabs)

🤖 Prompt for AI Agents
In share/icons/icon_set_from_svg.sh around lines 43 to 62, the script currently
outputs PNG icons directly under hicolor/<size>x<size>, flattening the category
structure. To fix this, modify the output directory path to include the category
subdirectory by extracting the category from the SVG file path relative to the
scalable directory and appending it to the output path, so icons are saved under
hicolor/<size>x<size>/<category>. Ensure the script creates these category
directories before generating the PNG files.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets hold off on the path for now need to determine best place for the script/svg as share/../.. is a production folder sugestions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Explore the repository structure to understand directory conventions
echo "=== Repository structure overview ==="
fd -t d -d 2 . | head -20

echo -e "\n=== Looking for build/generation scripts ==="
fd -t f -e sh . | grep -E "(build|generate|convert|compile)" | head -10

echo -e "\n=== Contents of tools directory ==="
ls -la tools/ 2>/dev/null | head -15

echo -e "\n=== Contents of workflow directory ==="
ls -la workflow/ 2>/dev/null | head -15

echo -e "\n=== Looking for other icon/asset related files ==="
fd -t f . | grep -iE "(icon|svg|png)" | head -20

echo -e "\n=== Check if there's a docs or assets directory ==="
ls -la docs/ 2>/dev/null | head -5
ls -la assets/ 2>/dev/null | head -5

Length of output: 2150


🏁 Script executed:

#!/bin/bash
# Look for SVG files specifically
echo "=== Looking for SVG files ==="
fd -e svg . | head -20

echo -e "\n=== Check staging directory contents ==="
ls -la staging/ 2>/dev/null | head -15

echo -e "\n=== Look for any shell scripts that might be utilities ==="
fd -e sh . -t f | head -20

echo -e "\n=== Check if there's a scripts or utils directory ==="
fd -t d . | grep -E "(script|util|tool|build)" | head -10

echo -e "\n=== Check the root directory for any build/make files ==="
ls -la | grep -E "(Makefile|build|script)" | head -10

Length of output: 1929


@Tearran, based on your project structure, here are my suggestions for organizing the icon generation script and SVG files:

For the script (icon_set_from_svg.sh):
Move it to SDK/ directory alongside your other development utilities. I see you already have similar tools there:

  • SDK/10_validate_module.sh
  • SDK/20_promote_module.sh
  • SDK/30_docstring.sh
  • etc.

Suggested new location: SDK/60_icon_set_from_svg.sh (following your numbering convention)

For the SVG source files:
You have two good options:

  1. Keep in share/ if you consider SVGs as production assets that should be distributed:

    • share/icons/hicolor/scalable/*.svg (current location)
  2. Create a source directory if you want clearer separation between source and generated files:

    • SDK/assets/icons/*.svg - keeps all build sources with SDK tools
    • Or src/assets/icons/*.svg - groups with other source code

The script would then reference the chosen SVG location and output PNGs to share/icons/hicolor/[size]/.

Would you prefer option 1 (SVGs as production assets in share/) or option 2 (SVGs as source files in SDK/assets/ or src/assets/)?


🧠 Learnings used
Learnt from: Tearran
PR: Tearran/configng-v2#52
File: tools/20_promote_module.sh:81-89
Timestamp: 2025-06-29T18:44:33.273Z
Learning: In tools/20_promote_module.sh, Tearran prefers fail-fast behavior when moving image files to ./docs/ - if the docs directory doesn't exist at promotion time, the script should fail rather than auto-create it, as this indicates an upstream workflow failure that needs to be addressed.

Learnt from: Tearran
PR: Tearran/configng-v2#0
File: :0-0
Timestamp: 2025-07-20T02:30:38.326Z
Learning: Tearran's configng-v2 project will grow to ~10 system modules and ~30 software modules (split 50/50 between docker and apt/git tools). The nesting optimization should only target sparse categories (network, localization) that won't justify grouping, while preserving necessary structure for dense categories that will need logical grouping due to volume.

Learnt from: Tearran
PR: Tearran/configng-v2#88
File: workflow/35_web_docs.sh:159-160
Timestamp: 2025-07-20T18:53:30.861Z
Learning: In workflow/35_web_docs.sh, Tearran prefers fail-fast behavior when writing to the modules_browsers directory. If the directory doesn't exist when json2docs tries to write to it, the script should fail rather than auto-create it with mkdir -p, as this indicates an upstream workflow failure that needs to be addressed.

Learnt from: Tearran
PR: Tearran/configng-v2#35
File: staging/cockpit.sh:4-6
Timestamp: 2025-06-23T06:57:50.987Z
Learning: In the configng-v2 project, files in the staging/ directory are temporary locations for PR review. The relative path comments in these staging files should reflect the final destination path where the file will be moved before merging, not the current staging location.

Learnt from: Tearran
PR: Tearran/configng-v2#35
File: src/software/system/cockpit.sh:86-93
Timestamp: 2025-06-23T07:08:44.775Z
Learning: For demo/prototype code in this repository, the user prefers to keep implementation less strict and avoid over-engineering, focusing on functionality rather than robustness of paths and sourcing.

Learnt from: Tearran
PR: Tearran/configng-v2#36
File: src/network/interface/network_manager.sh:46-53
Timestamp: 2025-06-23T18:54:21.952Z
Learning: Tearran prefers heredoc over echo/printf for multi-line templating work, especially for generating dynamic content from metadata. Heredoc provides better readability, easier maintenance, and is more forgiving for complex formatting than chained echo statements.

Learnt from: Tearran
PR: Tearran/configng-v2#71
File: tools/00_start_here.sh:243-246
Timestamp: 2025-07-15T07:35:26.542Z
Learning: Tearran prefers applying the scaffolding standard systematically across tools scripts, including .conf metadata files and function naming conventions, to create project-wide consistency between tools and modules.

Learnt from: Tearran
PR: Tearran/configng-v2#36
File: src/network/interface/network_manager.sh:46-53
Timestamp: 2025-06-23T18:48:41.687Z
Learning: Tearran prefers metadata-driven help generation using .conf files that can be parsed with basic bash tools (grep, sed, cat, jq). The .conf format contains rich metadata including feature names, descriptions, options, and requirements that should drive dynamic _about_ function generation rather than hardcoded help text.

Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T17:36:25.934Z
Learning: Tearran needs a registry system to track multiple helper functions (_*_*) beyond the main function and _about_* pattern for parsing and metadata purposes.

Learnt from: Tearran
PR: Tearran/configng-v2#0
File: :0-0
Timestamp: 2025-07-18T18:14:15.440Z
Learning: When Tearran suggests creating an issue for documentation updates instead of implementing them directly, he prefers this systematic approach for substantial changes that need focused planning and discussion, especially when it separates immediate PR concerns from broader improvements.

Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T15:56:11.653Z
Learning: Tearran is concerned about preventing false positive test patterns project-wide and wants systematic prevention rather than case-by-case fixes.

Learnt from: Tearran
PR: Tearran/configng-v2#52
File: tools/25_docstring.sh:46-56
Timestamp: 2025-06-29T19:11:03.193Z
Learning: When Tearran requests creating an issue for future enhancements that need separate focus, he prefers to acknowledge that the current code works as expected while planning the enhancement work separately rather than blocking current functionality.

Learnt from: Tearran
PR: Tearran/configng-v2#52
File: tools/25_docstring.sh:46-56
Timestamp: 2025-06-29T19:11:03.193Z
Learning: When Tearran requests creating an issue for future enhancements that need separate focus, he prefers to acknowledge that the current code works as expected while planning the enhancement work separately rather than blocking current functionality.

Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T21:50:00.575Z
Learning: Tearran's project currently uses only a 'contributor' field in configuration files, not a 'maintained' field. The contributor field appears to indicate both authorship and maintenance responsibility.

Learnt from: Tearran
PR: Tearran/configng-v2#43
File: tools/configng_v2.sh:14-17
Timestamp: 2025-06-25T03:11:52.041Z
Learning: When Tearran states their code works as expected, accept that at face value rather than assuming it needs fixes or has missing functionality.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (2)
SDK/60_icon_set_from_svg.sh (2)

54-59: Prefer testing the command directly over checking $?; add quality flags.

Reduces racey use of $? and improves rasterization quality for SVGs.

Note: Included in the larger diff above by converting to “if ; then … fi” and adding -density/-strip.


4-6: Expose CLI flags and align with project tooling conventions.

Given the project’s metadata-driven tooling preference, consider adding a .conf and minimal flag parsing now (e.g., --src, --sizes, --force, --dry-run), and track full scaffolding alignment as a separate issue so this PR can land.

I can draft: (1) a simple getopts parser to support --src/--sizes/--force/--dry-run, and (2) a matching .conf stub for help generation. Want me to open a follow-up issue and prepare the patch?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 73733b8 and 68eb976.

⛔ Files ignored due to path filters (6)
  • share/icons/hicolor/512x512/armbian_cpu_v2.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/armbian_font.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng-tux_v1.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng-tux_v2.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng_sprocket_v2.png is excluded by !**/*.png
  • share/icons/hicolor/512x512/configng_sproket_v1.png is excluded by !**/*.png
📒 Files selected for processing (1)
  • SDK/60_icon_set_from_svg.sh (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Tearran
PR: Tearran/configng-v2#84
File: docs/cockpit.md:7-8
Timestamp: 2025-07-18T17:27:45.451Z
Learning: The workflow/30_docstring.sh script correctly handles image copying during documentation generation. It finds images in module source directories (like src/software/user/cockpit.png), copies them to docs/, and generates correct relative markdown paths (./image.png). No manual intervention needed for image paths in auto-generated documentation.
Learnt from: Tearran
PR: Tearran/configng-v2#36
File: src/network/interface/network_manager.sh:46-53
Timestamp: 2025-06-23T18:54:21.952Z
Learning: Tearran prefers heredoc over echo/printf for multi-line templating work, especially for generating dynamic content from metadata. Heredoc provides better readability, easier maintenance, and is more forgiving for complex formatting than chained echo statements.
Learnt from: Tearran
PR: Tearran/configng-v2#71
File: tools/00_start_here.sh:243-246
Timestamp: 2025-07-15T07:35:26.542Z
Learning: Tearran prefers applying the scaffolding standard systematically across tools scripts, including .conf metadata files and function naming conventions, to create project-wide consistency between tools and modules.
Learnt from: Tearran
PR: Tearran/configng-v2#36
File: src/network/interface/network_manager.sh:46-53
Timestamp: 2025-06-23T18:48:41.687Z
Learning: Tearran prefers metadata-driven help generation using .conf files that can be parsed with basic bash tools (grep, sed, cat, jq). The .conf format contains rich metadata including feature names, descriptions, options, and requirements that should drive dynamic _about_ function generation rather than hardcoded help text.
Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T17:36:25.934Z
Learning: Tearran needs a registry system to track multiple helper functions (_*_*) beyond the main function and _about_* pattern for parsing and metadata purposes.
Learnt from: Tearran
PR: Tearran/configng-v2#0
File: :0-0
Timestamp: 2025-07-18T18:14:15.440Z
Learning: When Tearran suggests creating an issue for documentation updates instead of implementing them directly, he prefers this systematic approach for substantial changes that need focused planning and discussion, especially when it separates immediate PR concerns from broader improvements.
Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T15:56:11.653Z
Learning: Tearran is concerned about preventing false positive test patterns project-wide and wants systematic prevention rather than case-by-case fixes.
Learnt from: Tearran
PR: Tearran/configng-v2#52
File: tools/25_docstring.sh:46-56
Timestamp: 2025-06-29T19:11:03.193Z
Learning: When Tearran requests creating an issue for future enhancements that need separate focus, he prefers to acknowledge that the current code works as expected while planning the enhancement work separately rather than blocking current functionality.
Learnt from: Tearran
PR: Tearran/configng-v2#52
File: tools/25_docstring.sh:46-56
Timestamp: 2025-06-29T19:11:03.193Z
Learning: When Tearran requests creating an issue for future enhancements that need separate focus, he prefers to acknowledge that the current code works as expected while planning the enhancement work separately rather than blocking current functionality.
Learnt from: Tearran
PR: Tearran/configng-v2#23
File: staging/interface/test_info_box.sh:6-10
Timestamp: 2025-06-16T21:50:00.575Z
Learning: Tearran's project currently uses only a 'contributor' field in configuration files, not a 'maintained' field. The contributor field appears to indicate both authorship and maintenance responsibility.
Learnt from: Tearran
PR: Tearran/configng-v2#43
File: tools/configng_v2.sh:14-17
Timestamp: 2025-06-25T03:11:52.041Z
Learning: When Tearran states their code works as expected, accept that at face value rather than assuming it needs fixes or has missing functionality.
📚 Learning: 2025-07-18T17:27:45.451Z
Learnt from: Tearran
PR: Tearran/configng-v2#84
File: docs/cockpit.md:7-8
Timestamp: 2025-07-18T17:27:45.451Z
Learning: The workflow/30_docstring.sh script correctly handles image copying during documentation generation. It finds images in module source directories (like src/software/user/cockpit.png), copies them to docs/, and generates correct relative markdown paths (./image.png). No manual intervention needed for image paths in auto-generated documentation.

Applied to files:

  • SDK/60_icon_set_from_svg.sh
🔇 Additional comments (1)
SDK/60_icon_set_from_svg.sh (1)

1-62: Optional: update icon cache when targeting a live icon theme dir.

If this writes into an actual hicolor theme path used by the desktop, consider running gtk-update-icon-cache afterwards (often done in packaging post-install).

Would this script ever target a live system theme dir (e.g., /usr/share/icons/hicolor)? If yes, you may add:

  • gtk-update-icon-cache -f -t "$REPO_ROOT/share/icons/hicolor"

Comment on lines +1 to +7
#!/bin/bash

# Directory containing SVGs
SRC_DIR="share/icons/hicolor/scalable"
# List of desired sizes
SIZES=(16 32 64 128 256 512)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden script defaults (fail-fast, safe IFS).

Enable strict Bash modes to catch errors early and avoid subtle word-splitting bugs.

 #!/bin/bash

+set -Eeuo pipefail
+IFS=$'\n\t'
+
 # Directory containing SVGs
 SRC_DIR="share/icons/hicolor/scalable"
 # List of desired sizes
 SIZES=(16 32 64 128 256 512)
📝 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.

Suggested change
#!/bin/bash
# Directory containing SVGs
SRC_DIR="share/icons/hicolor/scalable"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
#!/bin/bash
set -Eeuo pipefail
IFS=$'\n\t'
# Directory containing SVGs
SRC_DIR="share/icons/hicolor/scalable"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
🤖 Prompt for AI Agents
In SDK/60_icon_set_from_svg.sh at lines 1 to 7, the script lacks strict Bash
settings that help catch errors early and prevent word-splitting issues. Add
'set -euo pipefail' at the start to enable fail-fast behavior and treat unset
variables as errors. Also, set 'IFS=$'\n\t'' to safely handle word splitting.
These changes will harden the script defaults for safer execution.

Comment on lines +4 to +6
SRC_DIR="share/icons/hicolor/scalable"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Make SRC_DIR robust to invocation path; allow override via flag/env.

The relative default breaks if the script isn’t run from repo root. Resolve relative to the script location and allow override.

Would you always run this from repo root? If not, adopt this change.

-# Directory containing SVGs
-SRC_DIR="share/icons/hicolor/scalable"
+# Directory containing SVGs (can override via SRC_DIR env/--src)
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
+REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
+SRC_DIR_DEFAULT="$REPO_ROOT/share/icons/hicolor/scalable"
+SRC_DIR="${SRC_DIR:-$SRC_DIR_DEFAULT}"

Also applies to: 28-33


🏁 Script executed:

#!/bin/bash
# Locate the script file
SCRIPT_PATH=$(fd --hidden --exclude .git icon_set_from_svg.sh)

echo "Found script at: $SCRIPT_PATH"
echo "----- First 100 lines of $SCRIPT_PATH -----"
sed -n '1,100p' "$SCRIPT_PATH"

Length of output: 2084


Ensure SRC_DIR is resolved from the script location and can be overridden

The current hard-coded relative path will fail when the script is run outside the repo root. Update the initialization of SRC_DIR to compute the repo root based on the script’s directory and honor an environment variable or CLI flag.

Locations to update:

  • SDK/60_icon_set_from_svg.sh, lines 4–6 (also applies similarly at lines 28–33)

Suggested diff:

- # Directory containing SVGs
- SRC_DIR="share/icons/hicolor/scalable"
+ # Resolve directory containing SVGs (overrideable via SRC_DIR env or --src flag)
+ SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
+ REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
+ SRC_DIR_DEFAULT="$REPO_ROOT/share/icons/hicolor/scalable"
+ SRC_DIR="${SRC_DIR:-$SRC_DIR_DEFAULT}"
📝 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.

Suggested change
SRC_DIR="share/icons/hicolor/scalable"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
# Resolve directory containing SVGs (overrideable via SRC_DIR env or --src flag)
SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
SRC_DIR_DEFAULT="$REPO_ROOT/share/icons/hicolor/scalable"
SRC_DIR="${SRC_DIR:-$SRC_DIR_DEFAULT}"
# List of desired sizes
SIZES=(16 32 64 128 256 512)
🤖 Prompt for AI Agents
In SDK/60_icon_set_from_svg.sh around lines 4 to 6, the SRC_DIR variable is set
to a hard-coded relative path which breaks if the script is run from outside the
repo root. Modify SRC_DIR to be dynamically resolved based on the script's
directory using a method like dirname and realpath, and allow it to be
overridden by an environment variable or CLI argument. Apply a similar update to
the related code at lines 28 to 33 to maintain consistency.

Comment on lines +9 to +26
if ! command -v convert &> /dev/null; then
echo "Error: ImageMagick 'convert' command not found."
read -p "Would you like to install ImageMagick using 'sudo apt install imagemagick'? [Y/n] " yn
case "$yn" in
[Yy]* | "" )
echo "Installing ImageMagick..."
sudo apt update && sudo apt install imagemagick
if ! command -v convert &> /dev/null; then
echo "Installation failed or 'convert' still not found. Exiting."
exit 1
fi
;;
* )
echo "Cannot proceed without ImageMagick. Exiting."
exit 1
;;
esac
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid interactive, distro-specific auto-install; support multiple converters.

Prompting for sudo apt in-script is non-portable (CI/non-Debian/macOS) and surprising. Detect available converters (magick/convert/rsvg-convert), fail with clear guidance, and support non-interactive runs.

-# Check for ImageMagick's convert command
-if ! command -v convert &> /dev/null; then
-	echo "Error: ImageMagick 'convert' command not found."
-	read -p "Would you like to install ImageMagick using 'sudo apt install imagemagick'? [Y/n] " yn
-	case "$yn" in
-		[Yy]* | "" )
-		echo "Installing ImageMagick..."
-		sudo apt update && sudo apt install imagemagick
-		if ! command -v convert &> /dev/null; then
-			echo "Installation failed or 'convert' still not found. Exiting."
-			exit 1
-		fi
-		;;
-		* )
-		echo "Cannot proceed without ImageMagick. Exiting."
-		exit 1
-	;;
-	esac
-fi
+# Resolve SVG->PNG converter once; prefer ImageMagick 7 (`magick`) then `convert`, then `rsvg-convert`.
+CONVERT_BIN=""
+if command -v magick >/dev/null 2>&1; then
+  CONVERT_BIN="magick convert"
+elif command -v convert >/dev/null 2>&1; then
+  CONVERT_BIN="convert"
+elif command -v rsvg-convert >/dev/null 2>&1; then
+  CONVERT_BIN="rsvg-convert"
+else
+  echo "Error: No SVG converter found. Install one of:"
+  echo "  - ImageMagick (magick/convert)"
+  echo "  - librsvg (rsvg-convert)"
+  echo "Tip (Debian/Ubuntu): sudo apt install imagemagick || sudo apt install librsvg2-bin"
+  echo "Tip (Fedora): sudo dnf install ImageMagick || sudo dnf install librsvg2-tools"
+  echo "Tip (macOS): brew install imagemagick || brew install librsvg"
+  exit 1
+fi
🤖 Prompt for AI Agents
In SDK/60_icon_set_from_svg.sh around lines 9 to 26, the script currently
prompts interactively to install ImageMagick using 'sudo apt install', which is
non-portable and unsuitable for CI or non-Debian systems. Refactor this section
to detect the presence of multiple possible converter commands like 'magick',
'convert', or 'rsvg-convert' without prompting. If none are found, exit with a
clear error message advising manual installation, ensuring the script can run
non-interactively and supports multiple platforms.

Comment on lines +35 to +41
shopt -s nullglob
svg_files=("$SRC_DIR"/*.svg)
if [ ${#svg_files[@]} -eq 0 ]; then
echo "Error: No SVG files found in '$SRC_DIR'."
exit 1
fi
shopt -u nullglob
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Support case-insensitive and recursive SVG discovery.

Catches .SVG and nested category subfolders (apps, mimetypes, etc.), which are common in hicolor themes.

-shopt -s nullglob
-svg_files=("$SRC_DIR"/*.svg)
+shopt -s nullglob nocaseglob
+# Recursively collect SVGs
+mapfile -t svg_files < <(find "$SRC_DIR" -type f -iname '*.svg' | sort)
 if [ ${#svg_files[@]} -eq 0 ]; then
 	echo "Error: No SVG files found in '$SRC_DIR'."
 	exit 1
 fi
-shopt -u nullglob
+shopt -u nullglob nocaseglob
📝 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.

Suggested change
shopt -s nullglob
svg_files=("$SRC_DIR"/*.svg)
if [ ${#svg_files[@]} -eq 0 ]; then
echo "Error: No SVG files found in '$SRC_DIR'."
exit 1
fi
shopt -u nullglob
shopt -s nullglob nocaseglob
# Recursively collect SVGs
mapfile -t svg_files < <(find "$SRC_DIR" -type f -iname '*.svg' | sort)
if [ ${#svg_files[@]} -eq 0 ]; then
echo "Error: No SVG files found in '$SRC_DIR'."
exit 1
fi
shopt -u nullglob nocaseglob
🤖 Prompt for AI Agents
In SDK/60_icon_set_from_svg.sh around lines 35 to 41, the current SVG file
discovery only matches .svg files in the top-level SRC_DIR and is
case-sensitive. Modify the script to recursively search all subdirectories under
SRC_DIR and include files with .svg and .SVG extensions by using a
case-insensitive and recursive find command. Replace the current globbing
approach with a find command that supports these requirements.

Comment on lines +43 to +62
# Loop over each SVG file in the scalable directory
for svg in "${svg_files[@]}"; do
# Extract the base filename without extension
base=$(basename "$svg" .svg)
# For each size, generate the PNG in the corresponding directory
for size in "${SIZES[@]}"; do
OUT_DIR="share/icons/hicolor/${size}x${size}"
mkdir -p "$OUT_DIR"
OUT_FILE="${OUT_DIR}/${base}.png"
# Only generate if missing or source SVG is newer
if [[ ! -f "$OUT_FILE" || "$svg" -nt "$OUT_FILE" ]]; then
convert -background none -resize ${size}x${size} "$svg" "$OUT_FILE"
if [ $? -eq 0 ]; then
echo "Generated $OUT_FILE"
else
echo "Failed to convert $svg to $OUT_FILE"
fi
fi
done
done
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Preserve theme subfolders and avoid filename collisions.

Flat output risks collisions across categories. Preserve the relative path under scalable and mirror it under size directories.

-# Loop over each SVG file in the scalable directory
-for svg in "${svg_files[@]}"; do
-	# Extract the base filename without extension
-	base=$(basename "$svg" .svg)
-	# For each size, generate the PNG in the corresponding directory
-	for size in "${SIZES[@]}"; do
-		OUT_DIR="share/icons/hicolor/${size}x${size}"
-		mkdir -p "$OUT_DIR"
-		OUT_FILE="${OUT_DIR}/${base}.png"
-		# Only generate if missing or source SVG is newer
-		if [[ ! -f "$OUT_FILE" || "$svg" -nt "$OUT_FILE" ]]; then
-			convert -background none -resize ${size}x${size} "$svg" "$OUT_FILE"
-		if [ $? -eq 0 ]; then
-			echo "Generated $OUT_FILE"
-		else
-			echo "Failed to convert $svg to $OUT_FILE"
-		fi
-	fi
-	done
-done
+# Loop over each SVG and mirror folder structure beneath size dirs
+for svg in "${svg_files[@]}"; do
+  rel="${svg#${SRC_DIR}/}"                 # e.g., apps/foo.svg
+  base_no_ext="${rel%.*}"                  # e.g., apps/foo
+  for size in "${SIZES[@]}"; do
+    OUT_DIR="$REPO_ROOT/share/icons/hicolor/${size}x${size}/$(dirname "$base_no_ext")"
+    mkdir -p "$OUT_DIR"
+    OUT_FILE="$OUT_DIR/$(basename "$base_no_ext").png"
+    # Only generate if missing or source SVG is newer
+    if [[ ! -f "$OUT_FILE" || "$svg" -nt "$OUT_FILE" ]]; then
+      if [[ "$CONVERT_BIN" == "rsvg-convert" ]]; then
+        # librsvg path
+        if rsvg-convert -w "$size" -h "$size" -o "$OUT_FILE" "$svg"; then
+          echo "Generated $OUT_FILE"
+        else
+          echo "Failed to convert $svg to $OUT_FILE"
+        fi
+      else
+        # ImageMagick path
+        if $CONVERT_BIN -background none -density 384 "$svg" -resize "${size}x${size}" -strip "$OUT_FILE"; then
+          echo "Generated $OUT_FILE"
+        else
+          echo "Failed to convert $svg to $OUT_FILE"
+        fi
+      fi
+    fi
+  done
+done

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In SDK/60_icon_set_from_svg.sh between lines 43 and 62, the script currently
outputs PNG files into flat size directories, risking filename collisions across
categories. Modify the script to preserve the relative directory structure of
each SVG file under the scalable directory and replicate that structure inside
each size directory. This involves computing the relative path of each SVG file
to the scalable root and creating corresponding subdirectories inside the
size-specific output directories before saving the PNG files there.

@Tearran Tearran merged commit 2d8e88f into main Aug 9, 2025
4 checks passed
@Tearran Tearran deleted the icons-script branch August 10, 2025 01:43
@coderabbitai coderabbitai bot mentioned this pull request Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants