Conversation
Reviewer's GuideThis PR restructures the monolithic dytoy scripts into cohesive, single-responsibility modules: package management is refactored under a unified pkg:: namespace, YAML metadata handling, archive extraction, binary download, service initialization, and miscellaneous utilities are each moved into dedicated libraries. Tool templates, docs, and configuration defaults are updated to match the new modular layout and reflect version bumps. Sequence diagram for installing a tool using dytoy with modular librariessequenceDiagram
participant User as actor User
participant dytoy as dytoy (main script)
participant dytoy_yaml as dytoy_yaml.sh
participant pkg as package_manager.sh
participant binary as binary_download.sh
participant compressed as compressed.sh
participant init as init_system.sh
participant misc as misc.sh
User->>dytoy: Request install tool (e.g. dytoy -t toolname)
dytoy->>dytoy_yaml: get_yaml(toolname, method)
dytoy_yaml->>pkg: Check if package is installed
alt Not installed
dytoy_yaml->>pkg: Install package (per OS)
pkg->>compressed: Extract archive if needed
pkg->>init: Enable service if needed
else Already installed
dytoy_yaml->>misc: Skip installation
end
dytoy_yaml->>User: Report success or error
ER diagram for dytoy tool metadata YAML structureerDiagram
TOOL {
string name
string method
bool is_essential
list dependencies
list packages
list archive
list services
string repo
string url
string repo_name
string repo_version
string distro_version
string key
string type
string unstable
string hook_before
string hook_after
}
TOOL ||--o{ PACKAGE : contains
TOOL ||--o{ SERVICE : enables
TOOL ||--o{ ARCHIVE : extracts
TOOL ||--o{ DEPENDENCY : depends_on
Class diagram for refactored dytoy package manager functionsclassDiagram
class pkg {
<<namespace>>
+sync_portage_repo()
+sync_pacman_repo()
+sync_apt_repo()
+sync_apk_repo()
+sync_termux_repo()
+sync_brew_repo()
+init_gentoo()
+init_arch()
+init_ubuntu()
+init_alpine()
+init_termux()
+init_flatpak()
+init_macos()
+add_overlay(name, url)
+add_apt_repo(name, code, distro_version, version, key)
+add_flatpak_repo(name, url)
+add_brew_tap(name)
+check_installed_portage(package)
+check_installed_pacman(package)
+check_installed_apt(package)
+check_installed_apk(package)
+check_installed_flatpak(package)
+check_installed_brew(package)
+check_installed_mas(app_id)
+check_installed_dmg(app_name)
+install_via_portage(package)
+install_via_pacman(package)
+install_via_apt(package)
+install_via_apk(package)
+install_via_termux(package)
+install_via_flatpak(app_id, repo)
+install_via_brew(package)
+install_via_mas(app_id)
+install_via_dmg(app_name, url)
}
Class diagram for new dytoy modular librariesclassDiagram
class dytoy_yaml {
+get_yaml(name, field)
+install_dependencies(name)
+run_script(script_file)
+create_script(name, path, content, kind)
+iterate(command)
+is_defined(name, method)
+is_invalid_essential(name)
+is_installed_command(name, location)
+is_installed_package(name, pkg_tool)
+enable_service(yaml, init_system)
+install_gentoo_package(yaml, init_system)
+install_arch_package(yaml)
+add_apt_repo(yaml, os)
+install_ubuntu_package(yaml)
+install_alpine_package(yaml)
+install_termux_package(yaml)
+install_flatpak_package(yaml)
+install_macos_package(yaml)
+install_macos_rosetta()
}
class compressed {
+extract_tar(name, path, location, url)
+extract_zip(name, path, location)
+extract_bzip2(name, path, location)
+extract_gzip(name, path, location)
}
class binary_download {
+get_latest_version(host, repo)
+download_and_extract(name, location, url)
}
class init_system {
+enable_systemd_service(service, is_user_service)
+enable_openrc_service(service, is_user_service)
+enable_termux_service(service)
+enable_launchd_service(service, is_user_service)
}
class misc {
+install_tool(name)
+replace_version(version)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @dynamotn - I've reviewed your changes - here's some feedback:
- pkg::sync_* and pkg::init_* functions across distros are almost identical—consider consolidating or using a data-driven approach to reduce boilerplate.
- The new scripts/lib modules introduce numerous global functions; splitting them into feature-specific files or namespaces will help maintain code organization.
- Function naming and argument ordering is inconsistent—establishing a clear naming convention and parameter structure will improve API clarity and reduce errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- pkg::sync_* and pkg::init_* functions across distros are almost identical—consider consolidating or using a data-driven approach to reduce boilerplate.
- The new scripts/lib modules introduce numerous global functions; splitting them into feature-specific files or namespaces will help maintain code organization.
- Function naming and argument ordering is inconsistent—establishing a clear naming convention and parameter structure will improve API clarity and reduce errors.
## Individual Comments
### Comment 1
<location> `scripts/lib/package_manager.sh:157` </location>
<code_context>
+ local name code distro_version version key
+ dybatpho::expect_args name code distro_version version key -- "$@"
+ local path
+ if ! dybatpho::is command termux-setup-storage; then
+ path="$PREFIX/etc/apt/sources.list.d/$name.list"
+ else
+ path="/etc/apt/sources.list.d/$name.list"
</code_context>
<issue_to_address>
Logic for Termux path detection appears inverted.
The current logic assigns the Termux path when 'termux-setup-storage' is absent, which is likely incorrect. Please ensure the path uses $PREFIX when running in Termux, as indicated by the presence of 'termux-setup-storage'.
</issue_to_address>
### Comment 2
<location> `scripts/lib/package_manager.sh:164` </location>
<code_context>
+ fi
+ if ! dybatpho::is file "$path"; then
+ dybatpho::debug "Adding repository $name."
+ dybatpho::dry_run eval "echo \"deb $code $distro_version $version\" | sudo tee \"/etc/apt/sources.list.d/$name.list\""
dybatpho::create_temp temp_key ".gpg"
dybatpho::dry_run dybatpho::curl_do "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x$key" "$temp_key"
</code_context>
<issue_to_address>
Hardcoded path in echo/tee may not match earlier path variable.
Use the `$path` variable in the `tee` command to ensure consistency with the earlier path assignment.
</issue_to_address>
### Comment 3
<location> `scripts/lib/package_manager.sh:184` </location>
<code_context>
+ if ! dybatpho::is command flatpak; then
+ dybatpho::die "Flatpak is not installed. Please install it first."
+ fi
+ if ! flatpak remote-list | grep -q "^$name"; then
+ dybatpho::progress "Adding Flatpak repository $name"
+ dybatpho::dry_run flatpak remote-add --user --if-not-exists "$name" "$url"
</code_context>
<issue_to_address>
Flatpak remote existence check may match partial names.
Using `^$name` in grep may match remotes with names that only start with `$name`. Use `^$name$` or another method to ensure an exact match and prevent false positives.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if ! flatpak remote-list | grep -q "^$name"; then
=======
if ! flatpak remote-list | grep -q "^$name$"; then
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `scripts/lib/package_manager.sh:261` </location>
<code_context>
+function pkg::check_installed_mas {
+ local app_id
+ dybatpho::expect_args app_id -- "$@"
+ mas list | grep -q "$app_id"
}
</code_context>
<issue_to_address>
App ID check may match partial IDs.
Consider using `grep -w` or a stricter pattern to avoid matching app IDs that are substrings of others.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
function pkg::check_installed_mas {
local app_id
dybatpho::expect_args app_id -- "$@"
=======
function pkg::check_installed_mas {
local app_id
dybatpho::expect_args app_id -- "$@"
mas list | grep -w -q "$app_id"
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `scripts/lib/package_manager.sh:377` </location>
<code_context>
+ dybatpho::curl_download "$url" "$temp_file"
+ local mount_dir
+ mount_dir=$(hdiutil mount -plist "$temp_file" | grep -oE '/Volumes/[^"<]+' | head -n 1)
+ sudo cp -r "${mount_dir}/${app_name}.app" /Applications
+ sudo hdiutil unmount "$mount_dir"
}
</code_context>
<issue_to_address>
No check for existing app before copying.
Copying without checking may overwrite an existing app. Please add a check for the app's existence and handle accordingly.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
sudo cp -r "${mount_dir}/${app_name}.app" /Applications
sudo hdiutil unmount "$mount_dir"
}
=======
if [ -e "/Applications/${app_name}.app" ]; then
echo "Warning: /Applications/${app_name}.app already exists. Skipping copy."
else
sudo cp -r "${mount_dir}/${app_name}.app" /Applications
fi
sudo hdiutil unmount "$mount_dir"
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dynamotn
force-pushed
the
dev
branch
2 times, most recently
from
8142b0e to
b2cc610
Compare
dynamotn
force-pushed
the
dev
branch
2 times, most recently
from
90de5be to
04be1c3
Compare
dynamotn
force-pushed
the
dev
branch
5 times, most recently
from
c377a2e to
efa757c
Compare
This was referenced Aug 11, 2025
dynamotn
force-pushed
the
dev
branch
7 times, most recently
from
5cca6d8 to
ebfd61c
Compare
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `scripts/lib/dytoy_yaml.sh:302` </location>
<code_context>
+# @description Install Alpine package from the YAML file
+# @arg $1 string YAML content
+#######################################
+function dytoy::install_alpine_package {
+ local yaml
+ dybatpho::expect_args yaml -- "$@"
+ local name=$(echo "$yaml" | yq e '.name')
+ if ! dytoy::is_installed_package "$name" "apk"; then
+ pkg::install_via_termux "$apk" \
+ && dybatpho::debug "Installed package: $name" \
+ && dytoy::enable_service "$yaml" "openrc" \
</code_context>
<issue_to_address>
Incorrect function used for Alpine package installation.
Please replace pkg::install_via_termux with pkg::install_via_apk to ensure proper installation on Alpine systems.
</issue_to_address>
### Comment 2
<location> `scripts/lib/dytoy_yaml.sh:318` </location>
<code_context>
+# @description Install Termux package from the YAML file
+# @arg $1 string YAML content
+#######################################
+function dytoy::install_termux_package {
+ local yaml
+ dybatpho::expect_args yaml -- "$@"
+ local name=$(echo "$yaml" | yq e '.name')
+ if ! dytoy::is_installed_package "$name" "apt"; then
+ pkg::install_via_termux "$name" \
+ && dybatpho::debug "Installed package: $name" \
</code_context>
<issue_to_address>
Termux package installation uses APT check instead of Termux-specific logic.
Using APT to check package installation may not work reliably in Termux. Please update the check to use Termux-specific logic for accurate results.
</issue_to_address>
### Comment 3
<location> `scripts/lib/dytoy_yaml.sh:352` </location>
<code_context>
+function dytoy::install_macos_package {
</code_context>
<issue_to_address>
Homebrew tap addition passes unused URL argument.
pkg::add_brew_tap is called with an extra "$url" argument, which is not expected by its definition. This may cause confusion or future errors if the function signature changes.
</issue_to_address>
### Comment 4
<location> `scripts/lib/binary_download.sh:11` </location>
<code_context>
+# @env GITHUB_TOKEN string Token for GitHub API
+# @env GITLAB_TOKEN string Token for GitLab API
+#######################################
+function binary::get_latest_version {
+ local host repo
+ dybatpho::expect_args host repo -- "$@"
+ dybatpho::create_temp temp_file ".txt"
+ dybatpho::debug "Get version ${name} from https://${host}/${repo}"
+ if [ "$type" = "github" ]; then
+ dybatpho::curl_do "https://api.${host}/repos/${repo}/releases/latest" "$temp_file" -H "'Authorization: Bearer ${GITHUB_TOKEN:-}'"
+ grep -Po "tag_name\": \"(\K.*)(?=\",)" "$temp_file"
+ elif [ "$type" = "gitlab" ]; then
</code_context>
<issue_to_address>
Authorization header for GitHub API is quoted incorrectly.
Remove the single quotes from the Authorization header to ensure curl interprets it correctly.
</issue_to_address>
### Comment 5
<location> `scripts/lib/compressed.sh:12` </location>
<code_context>
+function compressed:extract_tar {
</code_context>
<issue_to_address>
Use of --strip in tar may not be portable.
Not all tar versions support --strip; use --strip-components for broader compatibility or specify GNU tar as a requirement.
Suggested implementation:
```
local param=("--warning=no-unknown-keyword")
param+=("--wildcards")
```
```
if dybatpho::is true "$LIST_CONTENTS"; then
param+=("-vt")
else
param+=("-x")
fi
```
```
if [[ "$url" =~ \.(tar\.gz|tgz)$ ]]; then
```
If there are any lines in the function that use `--strip`, change them to use `--strip-components` instead.
If you want to ensure maximum compatibility, you may also want to document in your README or installation instructions that GNU tar is required, or add a check for tar version in your script.
</issue_to_address>
### Comment 6
<location> `scripts/lib/compressed.sh:74` </location>
<code_context>
+function compressed:extract_zip {
</code_context>
<issue_to_address>
Potential undefined variable 'version' in zip extraction.
'version' should be defined or passed to misc::replace_version to prevent runtime errors.
</issue_to_address>
### Comment 7
<location> `scripts/lib/init_system.sh:58` </location>
<code_context>
+# user-specific (true)
+# @arg $3 string Name of application (optional, used for user-specific services)
+#######################################
+function init::enable_launchd_service {
+ local service is_user_service
+ dybatpho::expect_args service is_user_service -- "$@"
+ dybatpho::progress "Enabling service $service"
+ if dybatpho::is true "$is_user_service"; then
+ /usr/bin/osascript -e "tell application \"System Events\" to make new login item at end with properties {name:\"${service}.app\", path:\"/Applications/${service}.app\", kind:\"Application\", hidden:true}"
+ else
+ brew services start "$service"
</code_context>
<issue_to_address>
Launchd service enabling uses osascript for user services, which may not be robust.
Consider replacing osascript with launchctl for enabling user services, as it provides more reliable and direct management of launchd services on MacOS.
</issue_to_address>
### Comment 8
<location> `README.md:77` </location>
<code_context>
+>
+> - If you aren't me, need to answer no (`n`) for "Do you want to decrypt... secrets?" questions.
+> Don't ask me why :)
+> - My chezmoi configuration are stored in [my private chezmoi repo for all machines](https://github.com/dynamotn/chezmoi-keeper)
## :scroll: Cheatsheet
</code_context>
<issue_to_address>
Subject-verb agreement error: 'My chezmoi configuration are stored' should be 'is stored'.
Use 'is stored' for correct subject-verb agreement.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
> - My chezmoi configuration are stored in [my private chezmoi repo for all machines](https://github.com/dynamotn/chezmoi-keeper)
=======
> - My chezmoi configuration is stored in [my private chezmoi repo for all machines](https://github.com/dynamotn/chezmoi-keeper)
>>>>>>> REPLACE
</suggested_fix>
### Comment 9
<location> `README.md:91` </location>
<code_context>
### via Mason of neovim
-All LSP servers, debuggers, formatters, linters when I used with `neovim` will be managed by [mason plugin](https://github.com/williamboman/mason.nvim/) and use my [custom registry](https://github.com/dynamotn/mason-registry/)
+All LSP servers, debuggers, formatters, linters when I used with `neovim` will be managed by [mason plugin](https://github.com/williamboman/mason.nvim/). I also have custom registry in `neovim` config.
</code_context>
<issue_to_address>
Missing article: 'I also have custom registry' should be 'I also have a custom registry'.
Add 'a' before 'custom registry' for grammatical correctness.
</issue_to_address>
### Comment 10
<location> `docs/dytoy.md:85` </location>
<code_context>
+|[xdg-user-dirs](https://www.freedesktop.org/wiki/Software/xdg-user-dirs/)|Manage well-known user directories|Prerequisite for GUI applications|
+|[hyprland](https://github.com/hyprwm/Hyprland)|Window manager for Wayland|`silos/hyprland`|
+|[hyprshade](https://github.com/loqusion/hyprshade)|Adjusts the color temperature of screen|`silos/hyprland`|
+|[hdrop](https://github.com/hyprwm/contrib#hdrop)|Dropdown utilities|`silos/hyrpland`|
+|[eww](https://github.com/elkowar/eww)|Widgets and status bar|`home/dot_config/eww`|
+|[wofi](https://sr.ht/~scoopta/wofi/)|Application launcher, simple switcher|`silos/hyprland`|
</code_context>
<issue_to_address>
Typo in Used in column: 'hyrpland' should be 'hyprland'.
Please update 'silos/hyrpland' to 'silos/hyprland' in the Used in column.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
|[hdrop](https://github.com/hyprwm/contrib#hdrop)|Dropdown utilities|`silos/hyrpland`|
=======
|[hdrop](https://github.com/hyprwm/contrib#hdrop)|Dropdown utilities|`silos/hyprland`|
>>>>>>> REPLACE
</suggested_fix>
### Comment 11
<location> `docs/dytoy.md:94` </location>
<code_context>
+|[regreet](https://github.com/rharish101/ReGreet)|ReGreet, greeter for GreetD|`root/etc/greetd`|
+|[hyprlock](https://github.com/hyprwm/hyprlock)|Screen locker for Wayland|`silos/hyprland`|
+|[swappy](https://github.com/jtheoof/swappy)|Editing snapshot tool for Wayland|`silos/hyprland`|
+|[grim](https://github.com/emersion/grim)|Screen capturing tool for Wayland|`silso/hyprland`|
+|[slurp](https://github.com/emersion/slurp)|Selecting region tool for Wayland|`silos/hyprland`|
+|[swaync](https://github.com/ErikReider/SwayNotificationCenter)|Notification center for Wayland|`silos/hyprland`|
</code_context>
<issue_to_address>
Typo in Used in column: 'silso' should be 'silos'.
Please update 'silso/hyprland' to 'silos/hyprland' for consistency.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
|[grim](https://github.com/emersion/grim)|Screen capturing tool for Wayland|`silso/hyprland`|
=======
|[grim](https://github.com/emersion/grim)|Screen capturing tool for Wayland|`silos/hyprland`|
>>>>>>> REPLACE
</suggested_fix>
### Comment 12
<location> `docs/dytoy.md:160` </location>
<code_context>
+
+|Tool|Purpose|Used in|
+|----|-------|-------|
+|[shdoc](https://github.com/mbucc/shmig)|Generate documents for Bash shell scripts|Development tool|
+|[watchexec](https://github.com/watchexec/watchexec)|Detects modifications and run command|Developer tool|
+|[gomplate](https://github.com/hairyhenderson/gomplate)|Template rendering, like jinja|Developer tool|
</code_context>
<issue_to_address>
Incorrect link: 'shdoc' is linked to 'shmig' repository.
Please change the link to reference the actual 'shdoc' repository.
</issue_to_address>
### Comment 13
<location> `docs/dytoy.md:258` </location>
<code_context>
+|[exiftool](https://github.com/exiftool/exiftool)|Meta information reader/writer for image|Multimedia tool|
+|[ImageMagick](https://github.com/ImageMagick/ImageMagick)|Image manipulation tool|Multimedia tool|
+|[hyperfine](https://github.com/sharkdp/hyperfine)|Benchmarking tool for CLI apps|Miscellaneous|
+|[cointop](https://github.com/arduino/arduino-cli)|TUI to track coin stats|Miscellaneous|
+|[tickrs](https://github.com/tarkah/tickrs)|TUI to track stock stats|Miscellaneous|
+|[pastel](https://github.com/sharkdp/pastel)|Generate, manipulate, convert colors CLI tool|Miscellaneous|
</code_context>
<issue_to_address>
Incorrect link: 'cointop' is linked to 'arduino-cli' repository.
Please change the link to 'https://github.com/marcusolsson/cointop'.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💻 Change Type
🔀 Description of Change
I want to manage dytoy scripts more flexible and split huge file to smaller functions, even a bit downgrade performance to maintain it easily
📝 Additional Information
Summary by Sourcery
Refactor and modularize the dytoy script suite by splitting the monolithic package manager logic into discrete pkg:: functions for each distro and package manager, introduce core dytoy libraries for YAML-driven tool installation and compressed/binary handling, and reorganize chezmoi tool templates and documentation for easier maintenance
Enhancements:
Documentation:
Chores: