diff --git a/.claude/agents/cdd-code-simplifier.md b/.claude/agents/cdd-code-simplifier.md index 154843d..34f7256 100644 --- a/.claude/agents/cdd-code-simplifier.md +++ b/.claude/agents/cdd-code-simplifier.md @@ -6,7 +6,7 @@ model: opus You are an expert code simplification specialist focused on enhancing code clarity, consistency, and maintainability while preserving exact functionality. Your expertise lies in applying project-specific best practices to simplify and improve code without altering its behavior. You prioritize readable, explicit code over overly compact solutions. -**Reference**: Follow the Bash Style Guide at https://style.ysap.sh/md +**Reference**: Follow the [Bash Style Guide](../../STYLEGUIDE.md) You will analyze recently modified code and apply refinements that: diff --git a/.claude/hooks/post-pr-simplify.sh b/.claude/hooks/post-pr-simplify.sh new file mode 100755 index 0000000..070e743 --- /dev/null +++ b/.claude/hooks/post-pr-simplify.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# +# PostToolUse hook: Trigger code simplifier after PR creation +# +# After a PR is successfully created, prompts Claude to run the +# cdd-code-simplifier agent against the changed files. + +# Debug log setup +debug_log="$HOME/.cache/claude-desktop-debian/hook-debug.log" +mkdir -p "$(dirname "$debug_log")" + +# Read JSON input from stdin - try cat as fallback +if [[ -t 0 ]]; then + printf '\n=== %s ===\nstdin is a terminal (no input)\n' "$(date)" >> "$debug_log" + exit 0 +fi + +input=$(cat) + +# Debug: log received input +printf '\n=== %s ===\ninput length: %d\n%s\n' "$(date)" "${#input}" "$input" >> "$debug_log" + +# Extract tool name, command, and response +tool_name=$(printf '%s' "$input" | jq -r '.tool_name // empty') +command=$(printf '%s' "$input" | jq -r '.tool_input.command // empty') + +# Try multiple paths for the response - Bash tool response structure may vary +response=$(printf '%s' "$input" | jq -r '.tool_response // empty') +stdout=$(printf '%s' "$input" | jq -r '.tool_response.stdout // .tool_response // empty') + +# Only process Bash tool calls +if [[ "$tool_name" != 'Bash' ]]; then + exit 0 +fi + +# Only process gh pr create commands +if [[ "$command" != *'gh pr create'* ]]; then + exit 0 +fi + +# Debug: log extracted values +printf 'tool_name=%s command=%s stdout=%s\n' "$tool_name" "$command" "$stdout" >> "$debug_log" + +# Check if the PR was created successfully (look for PR URL in output) +if [[ "$stdout" != *'github.com'* ]]; then + printf 'No github.com URL found in response, exiting\n' >> "$debug_log" + exit 0 +fi + +printf 'PR URL found, triggering simplifier\n' >> "$debug_log" + +# Get the list of changed files for context (as comma-separated list) +changed_files=$(git diff --name-only main...HEAD 2>/dev/null | head -20 | tr '\n' ', ' | sed 's/,$//') + +# Build the reason message (single line, no embedded newlines) +reason="PR created successfully. Now run the cdd-code-simplifier agent to review and simplify the code in this PR. Changed files: ${changed_files}. Use the Task tool with subagent_type='cdd-code-simplifier' to simplify the PR's changed code. After simplification, commit any changes and push to update the PR." + +# Output JSON to prompt Claude to run the simplifier +# Use jq to ensure valid JSON encoding +printf '%s\n' "$reason" | jq -Rs '{decision: "block", reason: .}' diff --git a/.claude/hooks/pre-pr-lint.sh b/.claude/hooks/pre-pr-lint.sh new file mode 100755 index 0000000..7bee812 --- /dev/null +++ b/.claude/hooks/pre-pr-lint.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# +# PreToolUse hook: Run shellcheck and actionlint before PR creation +# +# Checks shell scripts and GitHub Actions workflows for issues +# before allowing gh pr create to proceed. + +set -o pipefail + +# Read JSON input from stdin +input=$(/dev/null; then + echo 'Warning: shellcheck not installed, skipping shell script checks' >&2 + return + fi + + while IFS= read -r script; do + if [[ -f "$script" ]]; then + result=$(shellcheck -f gcc "$script" 2>&1) || true + if [[ -n "$result" ]]; then + errors+="shellcheck issues in $script:"$'\n'"$result"$'\n\n' + fi + fi + done <<< "$scripts" +} + +check_actionlint() { + local workflows="$1" + local workflow result + + if ! command -v actionlint &>/dev/null; then + echo 'Warning: actionlint not installed, skipping workflow checks' >&2 + return + fi + + while IFS= read -r workflow; do + if [[ -f "$workflow" ]]; then + result=$(actionlint "$workflow" 2>&1) || true + if [[ -n "$result" ]]; then + errors+="actionlint issues in $workflow:"$'\n'"$result"$'\n\n' + fi + fi + done <<< "$workflows" +} + +# Find modified shell scripts +changed_scripts=$(git diff --name-only main...HEAD 2>/dev/null | grep -E '\.sh$') || true +if [[ -n "$changed_scripts" ]]; then + check_shellcheck "$changed_scripts" +fi + +# Find modified workflow files +changed_workflows=$(git diff --name-only main...HEAD 2>/dev/null \ + | grep -E '\.github/workflows/.*\.ya?ml$') || true +if [[ -n "$changed_workflows" ]]; then + check_actionlint "$changed_workflows" +fi + +# If errors found, block the PR creation +if [[ -n "$errors" ]]; then + printf '%s\n' 'Lint checks failed. Fix these issues before creating the PR:' >&2 + printf '\n%s' "$errors" >&2 + exit 2 +fi + +# Report success +scripts_checked=0 +workflows_checked=0 +[[ -n "$changed_scripts" ]] && scripts_checked=$(printf '%s\n' "$changed_scripts" | wc -l) +[[ -n "$changed_workflows" ]] && workflows_checked=$(printf '%s\n' "$changed_workflows" | wc -l) + +printf 'Pre-PR lint check passed: %d shell scripts, %d workflows checked\n' \ + "$scripts_checked" "$workflows_checked" + +exit 0 diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 0000000..c9f9cad --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,28 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/pre-pr-lint.sh\"", + "timeout": 60 + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/post-pr-simplify.sh\"", + "timeout": 30 + } + ] + } + ] + } +} diff --git a/.claude/skills/simplify/SKILL.md b/.claude/skills/simplify/SKILL.md new file mode 100644 index 0000000..273808e --- /dev/null +++ b/.claude/skills/simplify/SKILL.md @@ -0,0 +1,30 @@ +--- +name: simplify +description: Manually trigger the cdd-code-simplifier agent to review and simplify code +disable-model-invocation: true +--- + +Run the cdd-code-simplifier agent to review and simplify code for clarity, consistency, and maintainability. + +## Your Task + +Use the Task tool with `subagent_type: "cdd-code-simplifier"` to run the code simplifier. + +**Scope determination:** + +1. If guidance was provided after `/simplify`, pass it to the agent as part of the prompt +2. If no guidance provided, have the agent focus on recently modified files (use `git diff --name-only main...HEAD` or `git status` to identify them) + +**User guidance:** $ARGUMENTS + +**After the agent completes:** + +1. Review the changes made +2. If changes were made, ask if the user wants to commit them +3. Provide a brief summary of what was simplified + +## Examples + +- `/simplify` - Simplify recently modified files +- `/simplify focus on build.sh error handling` - Simplify with specific guidance +- `/simplify only look at the new functions I added` - Narrow the scope diff --git a/CLAUDE.md b/CLAUDE.md index 63f0a10..3906a8e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,6 +4,16 @@ This project repackages Claude Desktop (Electron app) for Debian/Ubuntu Linux, applying necessary patches for Linux compatibility. +## Code Style + +All shell scripts in this project must follow the [Bash Style Guide](STYLEGUIDE.md). Key points: + +- Tabs for indentation, lines under 80 characters (exception: URLs and regex patterns) +- Use `[[ ]]` for conditionals, `$(...)` for command substitution +- Single quotes for literals, double quotes for expansions +- Lowercase variables; UPPERCASE only for constants/exports +- Use `local` in functions, avoid `set -e` and `eval` + ## GitHub Workflow ### General Approach diff --git a/STYLEGUIDE.md b/STYLEGUIDE.md new file mode 100644 index 0000000..5507966 --- /dev/null +++ b/STYLEGUIDE.md @@ -0,0 +1,598 @@ +Bash Style Guide +================ + +This guide outlines how to write bash scripts with a style that makes them safe +and predictable. This guide is written by [Dave Eddy](https://daveeddy.com) as +part of the YSAP (You Suck at Programming) series [ysap.sh](https://ysap.sh) and +is the working document for how I approach bash scripting when it comes to +style, design, and best-practices. + +Preface +------- + +This guide will try to be as objective as possible, providing reasoning for why +certain decisions were made. For choices that are purely aesthetic (and may +not be universally agreeable) they will exist in the `Aesthetics` section +below. + +Though good style alone won't ensure that your scripts are free from error, it +can certainly help narrow the scope for bugs to exist. This guide attempts to +explicitly state my style choices instead of implicitly relying on a sense or a +"vibe" of how code should be written. + +Aesthetics +---------- + +### Tabs / Spaces + +Tabs. + +### Columns + +Not to exceed 80, with exceptions for: +- URLs (keep them intact for copy/paste) +- Long regex patterns (breaking them reduces readability) + +### Semicolons + +Avoid using semicolons in scripts unless required in control statements (e.g., +if, while). + +``` bash +# wrong +name='dave'; +echo "hello $name"; + +# right +name='dave' +echo "hello $name" +``` + +The exception to this rule is outlined in the `Block Statements` section below. +Namely, semicolons should be used for control statements like `if` or `while`. + +### Functions + +Don't use the `function` keyword. All variables created in a function should +be made local. + +``` bash +# wrong +function foo { + i=foo # this is now global, wrong depending on intent +} + +# right +foo() { + local i=foo # this is local, preferred +} +``` + +### Block Statements + +`then` should be on the same line as `if`, and `do` should be on the same line +as `while`. + +``` bash +# wrong +if true +then + ... +fi + +# also wrong, though admittedly looks kinda cool +true && { + ... +} + +# right +if true; then + ... +fi +``` + +### Spacing + +No more than 2 consecutive newline characters (ie. no more than 1 blank line in +a row). + +### Comments + +No explicit style guide for comments. Don't change someones comments for +aesthetic reasons unless you are rewriting or updating them. + +--- + +Bashisms +-------- + +This style guide is for bash. This means when given the choice, always prefer +bash builtins or keywords instead of external commands or `sh(1)` syntax. + +### `test(1)` + +Use `[[ ... ]]` for conditional testing, not `[ .. ]` or `test ...` + +``` bash +# wrong +test -d /etc + +# also wrong +[ -d /etc ] + +# correct +[[ -d /etc ]] +``` + +- [YSAP066](https://ysap.sh/v/66) +See [BashFAQ031](http://mywiki.wooledge.org/BashFAQ/031) for more information +about these. + +### Sequences + +Use bash builtins for generating sequences + +``` bash +n=10 + +# wrong +for f in $(seq 1 5); do + ... +done + +# wrong +for f in $(seq 1 "$n"); do + ... +done + +# right +for f in {1..5}; do + ... +done + +# right +for ((i = 0; i < n; i++)); do + ... +done +``` + +- [YSAP052](https://ysap.sh/v/52/) +- [YSAP053](https://ysap.sh/v/53/) + +### Command Substitution + +Use `$(...)` for command substitution. + +``` bash +foo=`date` # wrong +foo=$(date) # right +``` + +- [YSAP022](https://ysap.sh/v/22/) + +### Math / Integer Manipulation + +Use `((...))` and `$((...))`. + +``` bash +a=5 +b=4 + +# wrong +if [[ $a -gt $b ]]; then + ... +fi + +# right +if ((a > b)); then + ... +fi +``` + +Do **not** use the `let` command. + +### Parameter Expansion + +Always prefer parameter expansion over external commands like `echo`, `sed`, +`awk`, etc. + +``` bash +name='bahamas10' + +# wrong +prog=$(basename "$0") +nonumbers=$(echo "$name" | sed -e 's/[0-9]//g') + +# right +prog=${0##*/} +nonumbers=${name//[0-9]/} +``` + +- [YSAP026](https://ysap.sh/v/26/) +- [YSAP056](https://ysap.sh/v/56/) + +### Listing Files + +Do not [parse ls(1)](http://mywiki.wooledge.org/ParsingLs), instead use +bash builtin functions to loop files + +``` bash +# very wrong, potentially unsafe +for f in $(ls); do + ... +done + +# right +for f in *; do + ... +done +``` + +- [YSAP001](https://ysap.sh/v/1/) + +### Determining path of the executable (`__dirname`) + +Simply stated, you can't know this for sure. If you are trying to find out the +full path of the executing program, you should rethink your software design. + +See [BashFAQ028](http://mywiki.wooledge.org/BashFAQ/028) for more information + +For a case study on `__dirname` in multiple languages see my blog post + +[Dirname Case +Study](http://daveeddy.com/2015/04/13/dirname-case-study-for-bash-and-node/) + +### Arrays and lists + +Use bash arrays instead of a string separated by spaces (or newlines, tabs, +etc.) whenever possible + +``` bash +# wrong +modules='json httpserver jshint' +for module in $modules; do + npm install -g "$module" +done + +# right +modules=(json httpserver jshint) +for module in "${modules[@]}"; do + npm install -g "$module" +done +``` + +Of course, in this example it may be better expressed as: + +``` bash +npm install -g "${modules[@]}" +``` + +... only if the command supports multiple arguments and you are not interested +in catching individual failures. + +- [YSAP020](https://ysap.sh/v/20) +- [Arrays explained in 7 minutes](https://www.youtube.com/watch?v=asHJ-xfuyno) + +### read builtin + +Use the bash `read` builtin whenever possible to avoid forking external +commands + +Example + +``` bash +fqdn='computer1.daveeddy.com' + +IFS=. read -r hostname domain tld <<< "$fqdn" +echo "$hostname is in $domain.$tld" +# => "computer1 is in daveeddy.com" +``` + +--- + +External Commands +----------------- + +### GNU userland tools + +The whole world doesn't run on GNU or on Linux; avoid GNU specific options +when forking external commands like `awk`, `sed`, `grep`, etc. to be as +portable as possible. + +When writing bash and using all the powerful tools and builtins bash gives you, +you'll find it rare that you need to fork external commands to do simple string +manipulation. + +- [YSAP029](https://ysap.sh/v/29/) + +### Useless Use of Cat Award + +Don't use `cat(1)` when you don't need it. If programs support reading from +stdin, pass the data in using bash redirection. + +``` bash +# wrong +cat file | grep foo + +# right +grep foo < file + +# also right +grep foo file +``` + +Prefer using a command line tools builtin method of reading a file instead of +passing in stdin. This is where we make the inference that, if a program says +it can read a file passed by name, it's probably more performant to do that. + +- [Your Using `cat` Wrong](https://www.youtube.com/watch?v=vAK55aiRLeY) +- [UUOC](http://www.smallo.ruhr.de/award.html) + +--- + +Style +----- + +### Quoting + +Use double quotes for strings that require variable expansion or command +substitution interpolation, and single quotes for all others. + +``` bash +# right +foo='Hello World' +bar="You are $USER" + +# wrong +foo="hello world" + +# possibly wrong, depending on intent +bar='You are $USER' +``` + +All variables that will undergo word-splitting *must* be quoted (1). If no +splitting will happen, the variable may remain unquoted. + +``` bash +foo='hello world' + +if [[ -n $foo ]]; then # no quotes needed: + # [[ ... ]] won't word-split variable expansions + + echo "$foo" # quotes needed +fi + +bar=$foo # no quotes needed - variable assignment doesn't word-split +``` + +1. The only exception to this rule is if the code or bash controls the variable +for the duration of its lifetime. For example code like this: + +``` bash +printf_date_supported=false +if printf '%()T' &>/dev/null; then + printf_date_supported=true +fi + +if $printf_date_supported; then + ... +fi +``` + +Even though `$printf_date_supported` undergoes word-splitting in the `if` +statement in that example, quotes are not used because the contents of that +variable are controlled explicitly by the programmer and not taken from a user +or command. + +Also, variables like `$$`, `$?`, `$#`, etc. don't required quotes because they +will never contain spaces, tabs, or newlines. + +When in doubt; [quote all expansions](http://mywiki.wooledge.org/Quotes). + +- [YSAP021](https://ysap.sh/v/21/) + +### Variable Declaration + +Avoid uppercase variable names unless they 1. are constants or 2. are exported +to the environment using `export`. Don't use `let` or `readonly` to create +variables. `declare` should *only* be used for associative arrays. `local` +should *always* be used in functions. + +``` bash +# wrong +declare -i foo=5 +let foo++ +readonly bar='something' +FOOBAR=baz + +# right +i=5 +((i++)) +bar='something' +export FOOBAR=baz +``` + +### shebang + +Bash is not always located at `/bin/bash`, so use this line: + +``` bash +#!/usr/bin/env bash +``` + +Unless you’re intentionally targeting a specific environment (e.g. `/bin/bash` +on Linux servers with restricted PATHs). + +- [Shebangs are Weird](https://www.youtube.com/watch?v=aoHMiCzqCNw) + +### Error Checking + +`cd`, for example, doesn't always work. Make sure to check for any possible +errors for `cd` (or commands like it) and exit or break if they are present. + +``` bash +# wrong +cd /some/path # this could fail +rm file # if cd fails where am I? what am I deleting? + +# right +cd /some/path || exit +rm file +``` + +### Using `set -e` + +Don't set `errexit`. Like in C, sometimes you want an error, or you expect +something to fail, and that doesn't necessarily mean you want the program +to exit. + +This is a controversial opinion that I have on the surface, but the link below +will show situations where `set -e` can do more harm than good because of its +implications. + +- [The Problem with Bash "strict mode"](https://www.youtube.com/watch?v=4Jo3Ml53kvc) +- [BashFAQ105](http://mywiki.wooledge.org/BashFAQ/105) + +### Using `eval` + +Never. + +It opens your code to code injection and makes static analysis impossible. +Almost every use-case can be solved more safely with arrays, indirect expansion, +or proper quoting. + +--- + +Common Mistakes +--------------- + +### Using {} instead of quotes. + +Using `${f}` is potentially different than `"$f"` because of how word-splitting +is performed. For example. + +``` bash +for f in '1 space' '2 spaces' '3 spaces'; do + echo ${f} +done +``` + +yields: + +``` +1 space +2 spaces +3 spaces +``` + +Notice that it loses the amount of spaces. This is due to the fact that the +variable is expanded and undergoes word-splitting because it is unquoted. This +loop results in the 3 following commands being executed: + +``` bash +echo 1 space +echo 2 spaces +echo 3 spaces +``` + +The extra spaces are effectively ignored here and only 2 arguments are passed +to the `echo` command in all 3 invocations. + +If the variable was quoted instead: + +``` bash +for f in '1 space' '2 spaces' '3 spaces'; do + echo "$f" +done +``` + +yields: + +``` +1 space +2 spaces +3 spaces +``` + +The variable `$f` is expanded but doesn't get split at all by bash, so it is +passed as a single string (with spaces) to the `echo` command in all 3 +invocations. + +Note that, for the most part `$f` is the same as `${f}` and `"$f"` is the same +as `"${f}"`. The curly braces should only be used to ensure the variable name +is expanded properly. For example: + +``` bash +$ echo "$HOME is $USERs home directory" +/home/dave is home directory +$ echo "$HOME is ${USER}s home directory" +/home/dave is daves home directory +``` + +The braces in this example were the difference of `$USER` vs `$USERs` being +expanded. + +### Abusing for-loops when while would work better + +`for` loops are great for iteration over arguments, or arrays. Newline +separated data is best left to a `while read -r ...` loop. + +``` bash +users=$(awk -F: '{print $1}' /etc/passwd) +for user in $users; do + echo "user is $user" +done +``` + +This example reads the entire `/etc/passwd` file to extract the usernames into +a variable separated by newlines. The `for` loop is then used to iterate over +each entry. + +This approach has a lot of issues if used on other files with data that may +contain spaces or tabs. + +1. This reads *all* usernames into memory, instead of processing them in a +streaming fashion. +2. If the first field of that file contained spaces or tabs, the for loop would +break on that as well as newlines. +3. This only works *because* `$users` is unquoted in the `for` loop - if +variable expansion only works for your purposes while unquoted this is a good +sign that something isn't implemented correctly. + +To rewrite this: + +``` bash +while IFS=: read -r user _; do + echo "$user is user" +done < /etc/passwd +``` + +This will read the file in a streaming fashion, not pulling it all into memory, +and will break on colons extracting the first field and discarding (storing as +the variable `_`) the rest - using nothing but bash builtin commands. + +- [YSAP038](https://ysap.sh/v/38/) + +--- + +References +---------- + +- [YSAP](https://ysap.sh) +- [BashGuide](https://mywiki.wooledge.org/BashGuide) +- [BashPitFalls](http://mywiki.wooledge.org/BashPitfalls) +- [Bash Practices](http://mywiki.wooledge.org/BashGuide/Practices) + +Get This Guide +-------------- + +- `curl style.ysap.sh` - View this guide in your terminal. +- `curl style.ysap.sh/plain` - View this guide without color in your terminal. +- `curl style.ysap.sh/md` - Get the raw markdown. +- [Website](https://style.ysap.sh) - Dedicated website for this guide. +- [GitHub](https://github.com/bahamas10/bash-style-guide) - View the source. + +License +------- + +MIT License