Skip to content

Add fixes - #3

Merged
JoannaaKL merged 5 commits into
mainfrom
improve-scripts
Sep 24, 2025
Merged

Add fixes#3
JoannaaKL merged 5 commits into
mainfrom
improve-scripts

Conversation

@JoannaaKL

Copy link
Copy Markdown
Owner

No description provided.

@JoannaaKL
JoannaaKL marked this pull request as ready for review September 22, 2025 09:21
Copilot AI review requested due to automatic review settings September 22, 2025 09:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Refactors the dotfiles bootstrap to be more robust and idempotent while adding automation and documentation. Key updates include a rewritten install script with safer patterns, modular helper functions, Codespaces management utilities, and CI ShellCheck linting.

  • Refactored install.sh with logging, preflight checks, safer symlinking, and tool installers
  • Added helper abstractions plus Codespaces export/cleanup scripts
  • Introduced ShellCheck CI workflow and comprehensive README

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
install.sh Rewritten bootstrap script with structured functions, logging, and idempotent installers
helpers.sh Introduces reusable install wrappers and environment-specific setup logic
export_codespace_cfg Adds managed SSH config export with block replacement logic
clean-codespaces.sh Adds script to prune shutdown Codespaces (supports dry run)
README.md Documents features, usage, and future ideas
.github/workflows/shellcheck.yml Adds CI workflow to lint shell scripts

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread helpers.sh Outdated
Comment thread install.sh Outdated
Comment thread install.sh Outdated
Comment on lines +46 to +50
shopt -s nullglob nocaseglob 2>/dev/null || true
local meslo_candidates=("$dest"/*Meslo*"Nerd Font"*.ttf)
if (( ${#meslo_candidates[@]} > 0 )); then
log "Meslo Nerd Font already present"; return 0; fi
shopt -u nocaseglob 2>/dev/null || true

Copilot AI Sep 22, 2025

Copy link

Choose a reason for hiding this comment

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

nullglob is enabled but never restored, changing global globbing semantics for the remainder of the script (could cause later globs to silently disappear). Save and restore original options or run the font detection in a subshell: ( shopt -s nullglob nocaseglob; meslo_candidates=(...); ... ).

Suggested change
shopt -s nullglob nocaseglob 2>/dev/null || true
local meslo_candidates=("$dest"/*Meslo*"Nerd Font"*.ttf)
if (( ${#meslo_candidates[@]} > 0 )); then
log "Meslo Nerd Font already present"; return 0; fi
shopt -u nocaseglob 2>/dev/null || true
local meslo_candidates_count
meslo_candidates_count=$(
( shopt -s nullglob nocaseglob
files=("$dest"/*Meslo*"Nerd Font"*.ttf)
echo "${#files[@]}"
)
)
if (( meslo_candidates_count > 0 )); then
log "Meslo Nerd Font already present"; return 0; fi

Copilot uses AI. Check for mistakes.
Comment thread install.sh Outdated
Comment thread install.sh Outdated
Comment on lines +79 to +84
local appimage_url="https://github.com/neovim/neovim/releases/latest/download/nvim.appimage"
local appimage_path="$SCRIPT_DIR/nvim.appimage"
curl -fsSL -o "$appimage_path" "$appimage_url"
chmod +x "$appimage_path"
"$appimage_path" --appimage-extract >/dev/null
ln -sfn "$SCRIPT_DIR/squashfs-root/usr/bin/nvim" "$HOME/bin/nvim"

Copilot AI Sep 22, 2025

Copy link

Choose a reason for hiding this comment

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

Extracting the AppImage into the repository directory (SCRIPT_DIR) pollutes the working tree with squashfs-root and the binary, potentially leading to accidental commits. Use a cache location like "$HOME/.local/share/neovim-appimage" or mktemp for extraction before symlinking.

Suggested change
local appimage_url="https://github.com/neovim/neovim/releases/latest/download/nvim.appimage"
local appimage_path="$SCRIPT_DIR/nvim.appimage"
curl -fsSL -o "$appimage_path" "$appimage_url"
chmod +x "$appimage_path"
"$appimage_path" --appimage-extract >/dev/null
ln -sfn "$SCRIPT_DIR/squashfs-root/usr/bin/nvim" "$HOME/bin/nvim"
local nvim_cache_dir="$HOME/.local/share/neovim-appimage"
mkdir -p "$nvim_cache_dir"
local appimage_url="https://github.com/neovim/neovim/releases/latest/download/nvim.appimage"
local appimage_path="$nvim_cache_dir/nvim.appimage"
if [[ ! -f "$appimage_path" ]]; then
curl -fsSL -o "$appimage_path" "$appimage_url"
chmod +x "$appimage_path"
fi
# Extract only if not already extracted
if [[ ! -d "$nvim_cache_dir/squashfs-root" ]]; then
(cd "$nvim_cache_dir" && "$appimage_path" --appimage-extract >/dev/null)
fi
ln -sfn "$nvim_cache_dir/squashfs-root/usr/bin/nvim" "$HOME/bin/nvim"

Copilot uses AI. Check for mistakes.
Comment thread export_codespace_cfg Outdated
JoannaaKL and others added 4 commits September 24, 2025 18:46
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
@JoannaaKL
JoannaaKL merged commit db7339f into main Sep 24, 2025
1 check failed
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