Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR syncs configuration changes from a development branch, primarily updating shell environment setup by replacing jenv with direct Java configuration, adding pyenv support, and introducing new utility functions and scripts.
Key Changes:
- Replaced jenv with direct Java 17 configuration and added pyenv initialization
- Added new utility functions (
dsa,kdsa,pyinit) and commented out nvm lazy loading - Introduced new Python initialization script and PDF sync utility
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| zsh/variables.zsh | Added OLLAMA_HOST environment variable for Ollama service configuration |
| zsh/source.zsh | Replaced jenv with direct JAVA_HOME setup and added pyenv initialization |
| zsh/functions.zsh | Fixed indentation, added dsa/kdsa/pyinit functions, commented out nvm lazy loading |
| zsh/.zshrc | Removed fzf-tab plugin and added Antigravity path configuration |
| scripts/shell/sync_pdf.sh | New script for converting markdown files to PDFs and syncing to iCloud |
| scripts/py/welcome.py | Improved code formatting with consistent indentation |
| scripts/py/init.py | New Python project initialization script that creates venv and project structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Kill Visual Studio Code | ||
| pkill -9 -f "code" |
There was a problem hiding this comment.
Using pkill -9 sends SIGKILL which doesn't allow processes to clean up gracefully. Consider using pkill without -9 first (which sends SIGTERM), and only use -9 as a fallback if the process doesn't terminate. This applies to the code kill command but not necessarily to Firefox and Obsidian.
| # Kill Visual Studio Code | |
| pkill -9 -f "code" | |
| # Kill Visual Studio Code gracefully, then force if needed | |
| pkill -f "code" | |
| sleep 2 | |
| pgrep -f "code" >/dev/null && pkill -9 -f "code" |
| export HISTFILE=~/.zsh_history | ||
| export SAVEHIST=1000000000 | ||
| export HISTDUP=erase | ||
| export OLLAMA_HOST="0.0.0.0:11434" |
There was a problem hiding this comment.
The OLLAMA_HOST is set to bind to all network interfaces (0.0.0.0), which exposes the Ollama service to the network without any authentication. This could be a security risk if the machine is on a shared network. Consider using "127.0.0.1:11434" to restrict access to localhost only, unless network access is intentionally required.
| export OLLAMA_HOST="0.0.0.0:11434" | |
| export OLLAMA_HOST="127.0.0.1:11434" |
| @@ -14,11 +14,14 @@ source "$DOTFILES/zsh/functions.zsh" | |||
| source "$ZSH/oh-my-zsh.sh" | |||
|
|
|||
| PATH="$PATH" | |||
There was a problem hiding this comment.
This line PATH="$PATH" is redundant as it assigns PATH to itself without any modification. This statement has no effect and should be removed.
| PATH="$PATH" |
| eval "$(pyenv init --path)" | ||
| eval "$(pyenv init -)" |
There was a problem hiding this comment.
Similar to the commented-out nvm lazy loading, pyenv initialization can slow down shell startup. Consider implementing lazy loading for pyenv as well, especially since nvm lazy loading was previously used in this codebase for the same reason.
| eval "$(pyenv init --path)" | |
| eval "$(pyenv init -)" | |
| # Lazy load pyenv initialization to speed up shell startup | |
| _pyenv_lazy_init() { | |
| # Only initialize pyenv once per session | |
| if [[ -z "$_PYENV_INITIALIZED" ]]; then | |
| export _PYENV_INITIALIZED=1 | |
| eval "$(pyenv init --path)" | |
| eval "$(pyenv init -)" | |
| fi | |
| } | |
| # Intercept common pyenv-related commands | |
| pyenv() { _pyenv_lazy_init; command pyenv "$@"; } | |
| python() { _pyenv_lazy_init; command python "$@"; } | |
| pip() { _pyenv_lazy_init; command pip "$@"; } |
| SOURCE_FOLDER="/Users/narayan/dump/one" | ||
|
|
||
| # Path where you want to generate PDFs temporarily | ||
| OUTPUT_FOLDER="/Users/narayan/dump/two" | ||
|
|
||
| # Final destination inside iCloud | ||
| ICLOUD_FOLDER="/Users/narayan/Library/Mobile Documents/com~apple~CloudDocs/Documents" |
There was a problem hiding this comment.
Hardcoded user-specific paths make this script non-portable. Consider using variables like $HOME or accepting these paths as command-line arguments or environment variables to make the script reusable by other users.
| SOURCE_FOLDER="/Users/narayan/dump/one" | |
| # Path where you want to generate PDFs temporarily | |
| OUTPUT_FOLDER="/Users/narayan/dump/two" | |
| # Final destination inside iCloud | |
| ICLOUD_FOLDER="/Users/narayan/Library/Mobile Documents/com~apple~CloudDocs/Documents" | |
| # Path where your original .md files are (can override via env) | |
| SOURCE_FOLDER="${SOURCE_FOLDER:-$HOME/dump/one}" | |
| # Path where you want to generate PDFs temporarily (can override via env) | |
| OUTPUT_FOLDER="${OUTPUT_FOLDER:-$HOME/dump/two}" | |
| # Final destination inside iCloud (can override via env) | |
| ICLOUD_FOLDER="${ICLOUD_FOLDER:-$HOME/Library/Mobile Documents/com~apple~CloudDocs/Documents}" |
| mkdir -p "$OUTPUT_FOLDER" | ||
|
|
||
| # Find all .md files recursively | ||
| find "$SOURCE_FOLDER" -type f -name "*.md" | while read mdfile; do |
There was a problem hiding this comment.
The read command should use the -r flag to prevent backslashes from being interpreted as escape characters. Use while read -r mdfile; do instead.
| find "$SOURCE_FOLDER" -type f -name "*.md" | while read mdfile; do | |
| find "$SOURCE_FOLDER" -type f -name "*.md" | while read -r mdfile; do |
| print(f"🔧 Running: {cmd}") | ||
| subprocess.run(cmd, shell=True, check=True, cwd=cwd) | ||
|
|
||
| def create_virtualenv(app_dir): | ||
| run("python3 -m venv venv", cwd=app_dir) |
There was a problem hiding this comment.
The run() function doesn't capture or return output, making it difficult to use for commands that need to return values. Additionally, using shell=True with user input can be a security risk. Consider using a list of arguments instead: subprocess.run(cmd.split(), check=True, cwd=cwd) or accept a list parameter.
| print(f"🔧 Running: {cmd}") | |
| subprocess.run(cmd, shell=True, check=True, cwd=cwd) | |
| def create_virtualenv(app_dir): | |
| run("python3 -m venv venv", cwd=app_dir) | |
| print(f"🔧 Running: {' '.join(cmd)}") | |
| subprocess.run(cmd, check=True, cwd=cwd) | |
| def create_virtualenv(app_dir): | |
| run(["python3", "-m", "venv", "venv"], cwd=app_dir) |
| export PATH="/opt/homebrew/opt/ruby/3.3.4/bin:$PATH" | ||
|
|
||
|
|
||
| export JAVA_HOME=$(/usr/libexec/java_home -v 17) |
There was a problem hiding this comment.
The command substitution for JAVA_HOME should be quoted to handle potential spaces or special characters in the path. Use export JAVA_HOME="$(/usr/libexec/java_home -v 17)" instead.
| export JAVA_HOME=$(/usr/libexec/java_home -v 17) | |
| export JAVA_HOME="$(/usr/libexec/java_home -v 17)" |
|
|
||
| # Sync OUTPUT_FOLDER to ICLOUD_FOLDER | ||
| echo "☁️ Syncing PDFs to iCloud..." | ||
| rsync -av --delete "$OUTPUT_FOLDER/" "$ICLOUD_FOLDER/" |
There was a problem hiding this comment.
The rsync command should check for errors after execution. Consider adding error handling: if [[ $? -ne 0 ]]; then echo "⚠️ Warning: rsync failed"; exit 1; fi
| rsync -av --delete "$OUTPUT_FOLDER/" "$ICLOUD_FOLDER/" | |
| rsync -av --delete "$OUTPUT_FOLDER/" "$ICLOUD_FOLDER/" | |
| if [[ $? -ne 0 ]]; then | |
| echo "⚠️ Warning: rsync failed" | |
| exit 1 | |
| fi |
| @@ -0,0 +1,74 @@ | |||
| import json | |||
| import os | |||
There was a problem hiding this comment.
Import of 'os' is not used.
| import os |
No description provided.