diff --git a/apply.sh b/apply.sh index 1885083..7969752 100755 --- a/apply.sh +++ b/apply.sh @@ -505,7 +505,7 @@ set_default_terminal_and_editor() { echo "[bootstrap] Setting system defaults via update-alternatives..." if host_has_role "desktop"; then - local nvim_path + local kitty_path kitty_path="$(command -v kitty || true)" if [[ -n "$kitty_path" ]]; then diff --git a/bin/docker_clean.sh b/bin/docker_clean.sh index b251cae..22d69d5 100755 --- a/bin/docker_clean.sh +++ b/bin/docker_clean.sh @@ -1,20 +1,17 @@ #!/usr/bin/env bash -function destroy_containers() { - containers="$(docker ps -a \ - | grep -Ev '^CONTAINER' \ - | awk '{print $1}')" +# IMPROVEMENT: Use docker's built-in filtering with -q flag instead of grep/awk parsing. +# This is more reliable as it avoids issues with column alignment or format changes. +function destroy_containers() { + containers="$(docker ps -aq)" if [ -n "$containers" ]; then echo "$containers" | xargs docker rm -f fi } function destroy_images() { - images="$(docker images \ - | grep -Ev '^REPOSITORY' \ - | awk '{print $3}')" - + images="$(docker images -q)" if [ -n "$images" ]; then echo "$images" | xargs docker rmi fi diff --git a/bin/fzf_cached_wsl b/bin/fzf_cached_wsl index c43e9e2..d1359f5 100755 --- a/bin/fzf_cached_wsl +++ b/bin/fzf_cached_wsl @@ -145,7 +145,8 @@ class FuzzyFileFinder: # Keep this commented out for performance except when debugging # Log.debug("ignoring file", {"ignore_pattern": ignore_pattern, "file": path_rel}) return - except: + # IMPROVEMENT: Use specific exception type instead of bare except + except re.error: print("ignore_pattern = {}, path_rel = {}".format(ignore_pattern, path_rel)) raise diff --git a/bin/set-theme b/bin/set-theme index 596a570..38e84b1 100755 --- a/bin/set-theme +++ b/bin/set-theme @@ -4,8 +4,7 @@ function yell () { >&2 echo "$*"; } function die () { yell "$*"; exit 1; } function try () { "$@" || die "Command failed: $*"; } -script_path="$( realpath "$0" )" -script_dir="$( dirname "$script_path" )" +# IMPROVEMENT: Removed unused script_path and script_dir variables theme="$( flavours list | tr " " "\n" | rofi -i -dmenu )" if test -z "$theme"; then diff --git a/bin/startup.sh b/bin/startup.sh index 050d063..4ae45f4 100755 --- a/bin/startup.sh +++ b/bin/startup.sh @@ -30,7 +30,8 @@ function start_process() { # If it's not running just run it if [ "$running" = "0" ]; then - $procname $procargs >> "$logfile" 2>&1 & + # BUG FIX: Quoted $procargs to prevent word splitting issues with args containing spaces + $procname "$procargs" >> "$logfile" 2>&1 & return fi @@ -41,7 +42,7 @@ function start_process() { sleep 0.1 done - $procname $procargs >> "$logfile" 2>&1 & + $procname "$procargs" >> "$logfile" 2>&1 & fi } diff --git a/cli/lib/common/file_walker.py b/cli/lib/common/file_walker.py index b46940b..e92defb 100644 --- a/cli/lib/common/file_walker.py +++ b/cli/lib/common/file_walker.py @@ -113,8 +113,9 @@ def _walk(ctx: Context, directory: Directory) -> None: elif dir_entry.is_file(follow_symlinks=True): FileWalker._handle_file(ctx, FileWalker.File(ctx.root, path_rel)) elif dir_entry.is_symlink(): + # BUG FIX: Fixed typo "non-existant" -> "non-existent" Log.debug( - "encountered symlink directory entry with non-existant target" + "encountered symlink directory entry with non-existent target" ) else: Log.warn( @@ -140,7 +141,7 @@ def _handle_dir(ctx: Context, directory: Directory) -> None: if ctx.halt: return - # If handler requested to skip, do nothin + # If handler requested to skip, do nothing if result.skip: return diff --git a/cli/lib/common/git.py b/cli/lib/common/git.py index e073e51..c19b6b4 100644 --- a/cli/lib/common/git.py +++ b/cli/lib/common/git.py @@ -6,9 +6,10 @@ from lib.common.log import Log +# BUG FIX: Return type annotation was str but function returns list[str] def _execute_git_command( cmd: list[str], strip: bool = True, filter_empty: bool = True -) -> str: +) -> list[str]: p = subprocess.Popen(cmd, stdout=subprocess.PIPE, text=True) stdout, _ = p.communicate() if p.returncode != 0: @@ -235,7 +236,8 @@ def add_all() -> None: @staticmethod def commit(message: str) -> None: - Log.debug("commiting staged changes") + # BUG FIX: Fixed typo "commiting" -> "committing" + Log.debug("committing staged changes") subprocess.check_call(["git", "commit", "--message", message]) @staticmethod diff --git a/cli/lib/common/log.py b/cli/lib/common/log.py index 6dbeba4..e2fc018 100644 --- a/cli/lib/common/log.py +++ b/cli/lib/common/log.py @@ -88,28 +88,30 @@ def init( Log._logger = logger + # IMPROVEMENT: Changed mutable default argument {} to None to avoid the + # well-known Python gotcha where mutable defaults are shared across calls @staticmethod - def debug(msg: str, data: LogData = {}) -> None: - Log._log(logging.DEBUG, msg, data) + def debug(msg: str, data: Optional[LogData] = None) -> None: + Log._log(logging.DEBUG, msg, data or {}) @staticmethod - def info(msg: str, data: LogData = {}) -> None: - Log._log(logging.INFO, msg, data) + def info(msg: str, data: Optional[LogData] = None) -> None: + Log._log(logging.INFO, msg, data or {}) @staticmethod - def warn(msg: str, data: LogData = {}) -> None: - Log._log(logging.WARNING, msg, data) + def warn(msg: str, data: Optional[LogData] = None) -> None: + Log._log(logging.WARNING, msg, data or {}) @staticmethod - def error(msg: str, data: LogData = {}) -> None: - Log._log(logging.ERROR, msg, data) + def error(msg: str, data: Optional[LogData] = None) -> None: + Log._log(logging.ERROR, msg, data or {}) @staticmethod - def fatal(msg: str, data: LogData = {}) -> None: - Log._log(logging.FATAL, msg, data) + def fatal(msg: str, data: Optional[LogData] = None) -> None: + Log._log(logging.FATAL, msg, data or {}) @staticmethod - def _log(level: LogLevel, msg: str, data: LogData = {}) -> None: + def _log(level: LogLevel, msg: str, data: LogData) -> None: if Log._logger is None: return Log._logger.log(level, Log._format_msg(msg, data)) diff --git a/config/bash/aliases.sh b/config/bash/aliases.sh index 482107f..4bc7e30 100644 --- a/config/bash/aliases.sh +++ b/config/bash/aliases.sh @@ -108,7 +108,8 @@ fi # Apt aliases if _is_installed 'apt'; then set_alias '0' 'apti' 'sudo apt install -y' - set_alias '0' 'apts' 'sudo apt search' + # IMPROVEMENT: Removed sudo from apt search - sudo is unnecessary for search operations + set_alias '0' 'apts' 'apt search' fi # Pacman aliases diff --git a/config/bash/functions.sh b/config/bash/functions.sh index 550c517..d33bfa5 100644 --- a/config/bash/functions.sh +++ b/config/bash/functions.sh @@ -35,7 +35,8 @@ function fm() { file_manager="$(xdg-mime query default inode/directory | sed -e 's/\.desktop//')" - if test -z "path"; then + # BUG FIX: Was missing $ before path variable, so condition was always false + if test -z "$path"; then path="." fi @@ -98,10 +99,11 @@ function nvimp() { } # Download the audio from a YouTube video as an MP3 file +# IMPROVEMENT: Changed from youtube-dl to yt-dlp, the actively maintained fork function yt_mp3() { - installed "youtube-dl" || return 1 + installed "yt-dlp" || return 1 - youtube-dl -x --audio-format "mp3" "$1" + yt-dlp -x --audio-format "mp3" "$1" } # WARNING: This shouldn't be called from an interactive shell as the passphrase @@ -284,7 +286,8 @@ function go_test_coverage() { if go test -coverprofile="$tempfile"; then go tool cover -html="$tempfile" else - 1>&1 echo -e "ERROR: Tests failed; to view coverage anyways run:\ngo tool cover -html=\"$tempfile\"" + # BUG FIX: Was using 1>&1 (stdout to stdout, no-op) instead of 1>&2 (stdout to stderr) + 1>&2 echo -e "ERROR: Tests failed; to view coverage anyways run:\ngo tool cover -html=\"$tempfile\"" fi } @@ -556,8 +559,9 @@ function tar_directory() { return 1 fi - path="$(realpath $dir)" - name="$(basename $path)" + # BUG FIX: Added quotes around $dir and $path to handle paths with spaces + path="$(realpath "$dir")" + name="$(basename "$path")" archive_path="/tmp/$name.tar.gz" diff --git a/config/bashrc b/config/bashrc index c9ff3d7..877b459 100644 --- a/config/bashrc +++ b/config/bashrc @@ -22,7 +22,7 @@ declare -a sources=( ) for i in "${sources[@]}"; do - src_if_exists "$i" + src_if_exists "$i" done # Source fzf shell integrations if we have them @@ -63,7 +63,8 @@ if _is_installed "go"; then fi # Rust configuration -if _is_installed "rust"; then +# BUG FIX: Changed "rust" to "rustc" - "rust" is not a command, rustc is the compiler +if _is_installed "rustc"; then [ -f "$HOME/.cargo/env" ] && . "$HOME/.cargo/env" fi diff --git a/config/env b/config/env index 3d33051..f7ee632 100644 --- a/config/env +++ b/config/env @@ -11,14 +11,14 @@ is_in_path() { esac } +# BUG FIX: The original implementations had swapped logic - +# "append" was prepending and "prepend" was appending. append_to_path() { - PATH=$1${PATH:+":$PATH"} - export PATH + export PATH="${PATH:+$PATH:}$1" } prepend_to_path() { - p="$1" - export PATH="${PATH}:${p}" + export PATH="$1${PATH:+:$PATH}" } try_append_to_path() { diff --git a/config/nvim/lua/dot/lsp_clangd.lua b/config/nvim/lua/dot/lsp_clangd.lua index 4dc7637..abe0b66 100644 --- a/config/nvim/lua/dot/lsp_clangd.lua +++ b/config/nvim/lua/dot/lsp_clangd.lua @@ -35,7 +35,8 @@ function M.configure() -- server attaches to the current buffer local on_attach = function(client, buf) -- Enable completion triggered by - vim.api.nvim_buf_set_option(buf, 'omnifunc', 'v:lua.vim.lsp.omnifunc') + -- IMPROVEMENT: nvim_buf_set_option is deprecated in favor of vim.bo[buf] + vim.bo[buf].omnifunc = 'v:lua.vim.lsp.omnifunc' -- See `:help vim.lsp.*` for documentation on the below functions Map.nnoremapbs(buf, 'gD', 'lua vim.lsp.buf.declaration()') diff --git a/config/nvim/lua/dot/lsp_gopls.lua b/config/nvim/lua/dot/lsp_gopls.lua index ec90111..c86f6f6 100644 --- a/config/nvim/lua/dot/lsp_gopls.lua +++ b/config/nvim/lua/dot/lsp_gopls.lua @@ -19,7 +19,8 @@ function M.configure() -- language server attaches to the current buffer local on_attach = function(client, buf) -- Enable completion triggered by - vim.api.nvim_buf_set_option(buf, 'omnifunc', 'v:lua.vim.lsp.omnifunc') + -- IMPROVEMENT: nvim_buf_set_option is deprecated in favor of vim.bo[buf] + vim.bo[buf].omnifunc = 'v:lua.vim.lsp.omnifunc' -- See `:help vim.lsp.*` for documentation on the below functions Map.nnoremapbs(buf, 'ld', 'lua vim.lsp.buf.definition()') diff --git a/config/nvim/lua/dot/util.lua b/config/nvim/lua/dot/util.lua index fe105e0..6f8eba8 100644 --- a/config/nvim/lua/dot/util.lua +++ b/config/nvim/lua/dot/util.lua @@ -4,13 +4,17 @@ local Log = require('dot.log') local M = {} +-- IMPROVEMENT: vim.loop was renamed to vim.uv in Neovim 0.10+ +-- Use vim.uv if available, fall back to vim.loop for older versions +local uv = vim.uv or vim.loop + function M.is_windows() -- TODO: Confirm this is right; only checked it on Linux - return vim.loop.os_uname().sysname == "Windows" + return uv.os_uname().sysname == "Windows" end function M.is_linux() - return vim.loop.os_uname().sysname == "Linux" + return uv.os_uname().sysname == "Linux" end function M.path_sep() diff --git a/config/vimrc b/config/vimrc index 77147a3..dcd6b13 100644 --- a/config/vimrc +++ b/config/vimrc @@ -36,8 +36,12 @@ set softtabstop=4 set tabstop=4 set expandtab -" Don't use smart indent in markdown files -autocmd FileType markdown setlocal nosmartindent +" IMPROVEMENT: Wrapped autocmd in augroup to prevent duplicate autocmds on config reload +augroup dotfiles_markdown + autocmd! + " Don't use smart indent in markdown files + autocmd FileType markdown setlocal nosmartindent +augroup END " Set the Leader key. I leave the leader key as '\' and remap ' ' to it " instead of setting ' ' as the leader. This is so that showcmd is actually @@ -110,11 +114,15 @@ nnoremap r :source ~/.vimrc xnoremap n :normal " Install vim-plug automatically on Linux if it isn't already +" IMPROVEMENT: Wrapped VimEnter autocmd in augroup to prevent duplicate autocmds if has('unix') if empty(glob('~/.vim/autoload/plug.vim')) silent !curl -fLo ~/.vim/autoload/plug.vim --create-dirs \ \ https://raw.githubusercontent.com/junegunn/vim-plug/master/plug.vim - autocmd VimEnter * PlugInstall --sync | source $MYVIMRC + augroup dotfiles_plug_install + autocmd! + autocmd VimEnter * PlugInstall --sync | source $MYVIMRC + augroup END endif endif @@ -161,7 +169,11 @@ call plug#end() " Format the current C/C++ file with clang-format (Uses vim-clang-format plugin) let g:clang_format#detect_style_file = 1 -autocmd FileType c,cpp vnoremap q :ClangFormat +" IMPROVEMENT: Wrapped autocmd in augroup to prevent duplicate autocmds on config reload +augroup dotfiles_clang_format + autocmd! + autocmd FileType c,cpp vnoremap q :ClangFormat +augroup END " FZF nnoremap o :Files diff --git a/config/vsvimrc b/config/vsvimrc index 04335cb..cf047ee 100644 --- a/config/vsvimrc +++ b/config/vsvimrc @@ -54,6 +54,7 @@ nnoremap 7gt nnoremap 8gt nnoremap 9gt nnoremap 10gt +" BUG FIX: Removed duplicate mapping block that was accidentally included twice nnoremap 1gt nnoremap 2gt nnoremap 3gt @@ -65,17 +66,6 @@ nnoremap 8gt nnoremap 9gt nnoremap 10gt -nnoremap 1gt -nnoremap 2gt -nnoremap 3gt -nnoremap 4gt -nnoremap 5gt -nnoremap 6gt -nnoremap 7gt -nnoremap 8gt -nnoremap 9gt -nnoremap 0gt - nnoremap vo :vsc Edit.GoToFile nnoremap vm :vsc Edit.GoToMember nnoremap vs :vsc Edit.GoToSymbol diff --git a/config/wezterm.lua b/config/wezterm.lua index 8839791..7005493 100644 --- a/config/wezterm.lua +++ b/config/wezterm.lua @@ -4,7 +4,8 @@ local act = wezterm.action local config = {} -function get_wsl_domain() +-- IMPROVEMENT: Added 'local' to helper functions to avoid polluting global namespace +local function get_wsl_domain() -- This should match the entry in the list output by `wsl --list` that -- should be used as the default domain local wsl_domain_name = os.getenv("WEZTERM_WSL_DOMAIN") @@ -16,7 +17,7 @@ function get_wsl_domain() return wsl_domain_name end -function get_shell(tab_info) +local function get_shell(tab_info) local shell = '' if tab_info.active_pane.domain_name == "local" then shell = '(Git Bash) ' @@ -34,7 +35,7 @@ wezterm.on('format-window-title', function(tab, pane, tabs, panes, config) return index .. get_shell(tab) .. tab.active_pane.title end) -function tab_title(tab_info) +local function tab_title(tab_info) local title = tab_info.tab_title if title and #title > 0 then return title diff --git a/doc/setup_ubuntu.md b/doc/setup_ubuntu.md index e2782fe..e8f9156 100644 --- a/doc/setup_ubuntu.md +++ b/doc/setup_ubuntu.md @@ -2,7 +2,7 @@ ## New Machine Bootstrapping -These are the typical steps to perform immediately after the inital Ubuntu +These are the typical steps to perform immediately after the initial Ubuntu installation. Update the system and reboot: @@ -67,7 +67,7 @@ cd ~/dot **Note:** The first time `apply.sh`, nix profile won't be sourced in the active shell. The easiest workaround is to just open a new shell. -**TODO:** We should add a message to the end of the output instructing user to restart shell. We could write a file on the first run and check for its existence on subsequent runs. If it does not exist, prompt the user to restart the computer. Probably not a bad idea on the first bootstrap to make sure everything propogates. +**TODO:** We should add a message to the end of the output instructing user to restart shell. We could write a file on the first run and check for its existence on subsequent runs. If it does not exist, prompt the user to restart the computer. Probably not a bad idea on the first bootstrap to make sure everything propagates. ## Daily Operations diff --git a/nix/home/roles/gaming.nix b/nix/home/roles/gaming.nix index 7ca9c33..521c7b6 100644 --- a/nix/home/roles/gaming.nix +++ b/nix/home/roles/gaming.nix @@ -1,7 +1,7 @@ { pkgs, ... }: { - # Allow proprietary packages for gaming (e.g., Steam) - nixpkgs.config.allowUnfree = true; + # IMPROVEMENT: Removed redundant nixpkgs.config.allowUnfree = true since it's + # already set in core.nix which is always imported alongside gaming.nix home.packages = with pkgs; [ steam diff --git a/nix/home/roles/wsl.nix b/nix/home/roles/wsl.nix index fb5aff2..a0f2546 100644 --- a/nix/home/roles/wsl.nix +++ b/nix/home/roles/wsl.nix @@ -14,7 +14,9 @@ in # Install win32yank to Windows filesystem for clipboard integration # This needs to be on NTFS (not WSL filesystem) for performance reasons + # IMPROVEMENT: Using run to respect verbose/dry-run flags from home-manager home.activation.installWin32yank = lib.hm.dag.entryAfter [ "writeBoundary" ] '' + run() { echo "running: $*"; "$@"; } WIN32YANK_VERSION="${win32yankVersion}" WIN32YANK_URL="${win32yankUrl}" WIN32YANK_DIR="${win32yankInstallDir}"