Fix weather geolocation and make word of day fully offline by bgreg · Pull Request #1 · bgreg/goodmorning-script

bgreg · 2025-12-22T22:26:51Z

Summary

Weather: Fix "not found:" issue by requiring explicit location config instead of unreliable IP geolocation
Word of Day: Replace API calls with macOS built-in dictionary (fully offline)
New Feature: Add --section flag to run individual sections without preflight/updates

Changes

Weather (`lib/app/display.sh`, `setup.sh`, `doctor.sh`)

Require GOODMORNING_WEATHER_LOCATION environment variable
Add location prompt during setup with automatic lowercase conversion
Display temperature in Fahrenheit (&u parameter)
Show helpful message when location not configured

Word of the Day (`lib/app/sections/word_of_day.sh`)

Use Swift/CoreServices to access macOS dictionary (DCSCopyTextDefinition)
Select word from /usr/share/dict/words based on day of year
Remove curl and jq dependencies for this section
Fully offline - no network calls needed

Section Flag (`goodmorning.sh`)

Add --section NAME argument to run a single section
Skips preflight checks, background updates, and banner
Example: ./goodmorning.sh --section weather
Fix zparseopts boolean flag parsing

Test plan

Run ./goodmorning.sh --section weather with location set
Run ./goodmorning.sh --section word (verify offline dictionary works)
Run ./goodmorning.sh --help to see new options
Run full script and verify all sections work
Run shellspec to verify tests pass

🤖 Generated with Claude Code

Weather improvements: - Require explicit GOODMORNING_WEATHER_LOCATION config (IP geolocation unreliable) - Add location prompt to setup.sh with automatic lowercase conversion - Display temperature in Fahrenheit (US units) - Update doctor.sh to warn when location not configured Word of the day improvements: - Replace API-based approach with macOS built-in dictionary (fully offline) - Use Swift/CoreServices to access DCSCopyTextDefinition - Select word from /usr/share/dict/words based on day of year - Remove curl/jq dependencies for this section New --section flag: - Add --section NAME to run individual sections without preflight/updates - Useful for testing and quick lookups (e.g., --section weather) - Fix zparseopts boolean checks using ${+opts[key]} syntax 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR addresses reliability issues with weather geolocation and makes the word of the day feature fully offline by using macOS built-in dictionary. It also adds a convenience flag to run individual sections.

Key changes:

Weather now requires explicit GOODMORNING_WEATHER_LOCATION configuration instead of unreliable IP geolocation
Word of Day replaces Dictionary API with Swift/CoreServices for offline dictionary lookups
New --section flag allows running individual sections without preflight checks

Reviewed changes

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

Show a summary per file

File	Description
lib/app/display.sh	Add location requirement and validation for weather feature
lib/app/sections/word_of_day.sh	Replace API calls with macOS dictionary via Swift/CoreServices
goodmorning.sh	Add `--section` flag support and fix zparseopts boolean checks
setup.sh	Add location prompt during weather setup with automatic lowercase conversion
lib/setup/doctor.sh	Update weather validation to check for location configuration
spec/lib/display_spec.sh	Add tests for weather location requirement
README.md	Document new `GOODMORNING_WEATHER_LOCATION` variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-22T22:45:34Z

@bgreg I've opened a new pull request, #2, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2025-12-22T22:46:24Z

@bgreg I've opened a new pull request, #3, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2025-12-22T22:46:58Z

@bgreg I've opened a new pull request, #4, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2025-12-22T22:47:37Z

@bgreg I've opened a new pull request, #5, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Use last element instead of hardcoded index 2, add validation for array length, and provide clear error message when --section value is missing. Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Make zparseopts section argument extraction more robust

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Document --section flag in README

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Replace grep-based weather tests with runtime behavior tests

Optimize word of day fallback to single Swift invocation

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-22T23:09:43Z

+  # Remove everything up to and including the part of speech, stop at DERIVATIVES/ORIGIN
+  def_text=$(echo "$definition" | sed -E "s/^.*$part_of_speech //" | sed -E 's/(DERIVATIVES|ORIGIN|PHRASES|•).*$//' | head -c 250 | xargs)


The sed command will fail if part_of_speech is empty. When part_of_speech is not found (line 83), it becomes an empty string, and then the sed pattern s/^.*$part_of_speech // becomes s/^.* // which will incorrectly remove everything up to the last space in the definition. This would result in truncated or incorrect definition text.

Consider checking if part_of_speech is non-empty before attempting to use it in the sed substitution, or use a different approach to extract the definition text.

Suggested change

# Remove everything up to and including the part of speech, stop at DERIVATIVES/ORIGIN

def_text=$(echo "$definition" | sed -E "s/^.*$part_of_speech //" | sed -E 's/(DERIVATIVES|ORIGIN|PHRASES|•).*$//' | head -c 250 | xargs)

# Remove everything up to and including the part of speech (if available), stop at DERIVATIVES/ORIGIN

local def_source

if [[ -n "$part_of_speech" ]]; then

def_source=$(echo "$definition" | sed -E "s/^.*$part_of_speech //")

else

# Fallback: use full definition text when part of speech is not detected

def_source="$definition"

fi

def_text=$(echo "$def_source" | sed -E 's/(DERIVATIVES|ORIGIN|PHRASES|•).*$//' | head -c 250 | xargs)

Copilot · 2025-12-22T23:09:43Z

+  fi
+
+  # Validate location to avoid unsafe characters in the URL
+  if ! [[ "$location" =~ ^[A-Za-z0-9_-]+$ ]]; then


The validation regex allows uppercase letters (A-Z), but the setup script (setup.sh:610) explicitly converts the location to lowercase using ${location_input:l}. This creates an inconsistency where the setup prevents uppercase input from ever being stored, yet the validation allows it.

Either remove A-Z from the regex to match the actual expected format, or remove the lowercase conversion in setup.sh if uppercase should be supported.

Suggested change

if ! [[ "$location" =~ ^[A-Za-z0-9_-]+$ ]]; then

if ! [[ "$location" =~ ^[a-z0-9_-]+$ ]]; then

Copilot · 2025-12-22T23:09:43Z

+              [[ "$arg" == "curl" ]] && has_curl=true
+              [[ "$arg" == "-s" ]] && has_silent=true
+              [[ "$arg" == "--max-time" ]] && has_timeout=true
+              [[ "$arg" == *"wttr.in/Boston"* ]] && has_url=true


The test verifies that the URL contains the location (wttr.in/Boston) but doesn't verify the presence of the &u parameter which was added to display temperature in Fahrenheit. Consider enhancing the test to check for the complete URL pattern including query parameters to ensure the Fahrenheit display works as intended.

Copilot · 2025-12-22T23:09:43Z

 ###############################################################################
 fetch_word_of_day() {
  local dict_file="/usr/share/dict/words"
  local day_of_year=$(date +%j | sed 's/^0*//')


If date +%j returns all zeros (edge case on some systems), the sed 's/^0*//' command would produce an empty string, making day_of_year empty. This would cause arithmetic errors in line 31 where day_of_year is used in the calculation.

While unlikely in practice, adding a fallback would make the code more robust:

local day_of_year=$(date +%j | sed 's/^0*//') [[ -z "$day_of_year" ]] && day_of_year=1

Suggested change

local day_of_year=$(date +%j | sed 's/^0*//')

local day_of_year=$(date +%j | sed 's/^0*//')

[[ -z "$day_of_year" ]] && day_of_year=1

Copilot · 2025-12-22T23:09:44Z

-    show_new_line
-    echo_gray "  Example: \"$example\""
-  fi
+  echo "$definition" | fold -s -w 70 | sed 's/^/  /'


Using echo without quotes can interpret escape sequences and special characters in the definition text. If the macOS dictionary definition contains backslashes, newlines, or other escape sequences, they may be interpreted unexpectedly.

Consider using printf '%s\n' "$definition" instead, which treats the content literally and is more portable.

Copilot · 2025-12-22T23:09:44Z

+
+  # Count matching words to ensure selection index is within bounds
+  local total_words selected_index
+  total_words=$(grep -E '^[a-z]{7,12}$' "$dict_file" 2>/dev/null | wc -l | tr -d ' ')
+
+  if [[ -n "$total_words" && "$total_words" -gt 0 ]]; then
+    # Compute a stable index in the range [1, total_words]
+    selected_index=$(( (day_of_year * 7) % total_words + 1 ))
+    word=$(grep -E '^[a-z]{7,12}$' "$dict_file" 2>/dev/null | awk "NR == $selected_index" | head -1)


The dictionary file is grepped twice with the same pattern - once to count words (line 27) and again to select a word (line 32). This is inefficient, especially since /usr/share/dict/words can be quite large (hundreds of thousands of words).

Consider storing the filtered words in a variable or using a single grep with awk to both count and select, or use a more efficient approach like:

local filtered_words=$(grep -E '^[a-z]{7,12}$' "$dict_file" 2>/dev/null) local total_words=$(echo "$filtered_words" | wc -l | tr -d ' ') ... word=$(echo "$filtered_words" | awk "NR == $selected_index")

Suggested change

# Count matching words to ensure selection index is within bounds

local total_words selected_index

total_words=$(grep -E '^[a-z]{7,12}$' "$dict_file" 2>/dev/null | wc -l | tr -d ' ')

if [[ -n "$total_words" && "$total_words" -gt 0 ]]; then

# Compute a stable index in the range [1, total_words]

selected_index=$(( (day_of_year * 7) % total_words + 1 ))

word=$(grep -E '^[a-z]{7,12}$' "$dict_file" 2>/dev/null | awk "NR == $selected_index" | head -1)

local -a filtered_words

# Get all matching words once, then count and select from the array

filtered_words=("${(@f)$(grep -E '^[a-z]{7,12}$' "$dict_file" 2>/dev/null)}")

local total_words selected_index

total_words=${#filtered_words[@]}

if [[ -n "$total_words" && "$total_words" -gt 0 ]]; then

# Compute a stable index in the range [1, total_words]

selected_index=$(( (day_of_year * 7) % total_words + 1 ))

word=${filtered_words[selected_index]}

Copilot · 2025-12-22T23:09:44Z

 | `GOODMORNING_LOGS_DIR` | Directory for log files | `$GOODMORNING_CONFIG_DIR/logs` | No |
 | `GOODMORNING_OUTPUT_HISTORY_DIR` | Directory for output history | `$GOODMORNING_CONFIG_DIR/output_history` | No |
 | `GOODMORNING_SHOW_WEATHER` | Show weather section | `true` | No |
+| `GOODMORNING_WEATHER_LOCATION` | City for weather (e.g., San_Francisco, London) | (none) | **Yes** |


The documentation marks GOODMORNING_WEATHER_LOCATION as "Required" with Yes, but this is misleading. The script runs successfully without this variable set - it simply skips the weather display with a helpful message (lib/app/display.sh:39-41).

The "Required" column should be "No" or the description should clarify it's "Required if weather is enabled" to accurately reflect the optional nature of this configuration.

Suggested change

| `GOODMORNING_WEATHER_LOCATION` | City for weather (e.g., San_Francisco, London) | (none) | **Yes** |

| `GOODMORNING_WEATHER_LOCATION` | City for weather (e.g., San_Francisco, London) | (none) | If weather enabled (`GOODMORNING_SHOW_WEATHER=true`) |

Copilot · 2025-12-22T23:09:44Z

+  word=$(echo "$word_data" | cut -f1)
+  phonetic=$(echo "$word_data" | cut -f2)
+  part_of_speech=$(echo "$word_data" | cut -f3)
+  definition=$(echo "$word_data" | cut -f4-)


Tab-separated value parsing with cut will fail if any of the fields (word, phonetic, part_of_speech, or definition) contain tab characters. Since these values come from the macOS dictionary, they could potentially include tabs in the definition text, causing field misalignment.

Consider using a more robust delimiter like <<<SEP>>> or escaping/replacing tabs in the output at line 90 before parsing here.

…ents Fix weather geolocation and make word of day fully offline

bgreg requested a review from Copilot December 22, 2025 22:36

Copilot started reviewing on behalf of bgreg December 22, 2025 22:37 View session

Copilot AI reviewed Dec 22, 2025

View reviewed changes

Initial plan

40c72f6

Copilot AI mentioned this pull request Dec 22, 2025

Make zparseopts section argument extraction more robust #2

Merged

bgreg and others added 3 commits December 22, 2025 14:45

Update lib/app/sections/word_of_day.sh

f15a0e5

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update goodmorning.sh

f14e2c8

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Initial plan

e988ed1

Copilot AI mentioned this pull request Dec 22, 2025

Document --section flag in README #3

Merged

bgreg and others added 2 commits December 22, 2025 14:46

Update lib/app/sections/word_of_day.sh

7ab96dc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Initial plan

9e44650

Copilot AI mentioned this pull request Dec 22, 2025

Replace grep-based weather tests with runtime behavior tests #4

Merged

bgreg and others added 4 commits December 22, 2025 14:47

Update lib/app/display.sh

ada7e51

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update lib/app/sections/word_of_day.sh

fd2af2d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Initial plan

e6faffc

Update lib/app/sections/word_of_day.sh

954558b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Dec 22, 2025

Optimize word of day fallback to single Swift invocation #5

Merged

Copilot AI and others added 8 commits December 22, 2025 22:48

Add documentation for --section flag in README

9a3b70d

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Make zparseopts section extraction more robust

484601f

Use last element instead of hardcoded index 2, add validation for array length, and provide clear error message when --section value is missing. Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Optimize word of day fallback to use single Swift invocation

18ebb03

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Address code review feedback: fix whitespace and improve parsing

4d61912

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Merge pull request #2 from bgreg/copilot/sub-pr-1

0708c13

Make zparseopts section argument extraction more robust

Improve awk parsing patterns for better robustness

b00d902

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Merge pull request #3 from bgreg/copilot/sub-pr-1-again

0861841

Document --section flag in README

Replace grep-based weather tests with runtime behavior tests

15d9ad4

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Copilot AI and others added 3 commits December 22, 2025 22:59

Add validation to basic mock to verify curl command is passed

0643f38

Co-authored-by: bgreg <3711139+bgreg@users.noreply.github.com>

Merge pull request #4 from bgreg/copilot/sub-pr-1-another-one

c66c9b5

Replace grep-based weather tests with runtime behavior tests

Merge pull request #5 from bgreg/copilot/sub-pr-1-yet-again

b30bc30

Optimize word of day fallback to single Swift invocation

bgreg self-assigned this Dec 22, 2025

bgreg requested a review from Copilot December 22, 2025 23:02

Copilot started reviewing on behalf of bgreg December 22, 2025 23:03 View session

bgreg merged commit e6a00e0 into main Dec 22, 2025
5 checks passed

Copilot AI reviewed Dec 22, 2025

View reviewed changes

bgreg added a commit that referenced this pull request Mar 31, 2026

Merge pull request #1 from bgreg/fix/weather-and-word-of-day-improvem…

36d5dea

…ents Fix weather geolocation and make word of day fully offline

bgreg added a commit that referenced this pull request Jun 30, 2026

Merge pull request #1 from bgreg/fix/weather-and-word-of-day-improvem…

eab7a67

…ents Fix weather geolocation and make word of day fully offline

		# Remove everything up to and including the part of speech, stop at DERIVATIVES/ORIGIN
		def_text=$(echo "$definition" \| sed -E "s/^.$part_of_speech //" \| sed -E 's/(DERIVATIVES\|ORIGIN\|PHRASES\|•).$//' \| head -c 250 \| xargs)

-  # Remove everything up to and including the part of speech, stop at DERIVATIVES/ORIGIN
-  def_text=$(echo "$definition" | sed -E "s/^.*$part_of_speech //" | sed -E 's/(DERIVATIVES|ORIGIN|PHRASES|•).*$//' | head -c 250 | xargs)
+  # Remove everything up to and including the part of speech (if available), stop at DERIVATIVES/ORIGIN
+  local def_source
+  if [[ -n "$part_of_speech" ]]; then
+    def_source=$(echo "$definition" | sed -E "s/^.*$part_of_speech //")
+  else
+    # Fallback: use full definition text when part of speech is not detected
+    def_source="$definition"
+  fi
+  def_text=$(echo "$def_source" | sed -E 's/(DERIVATIVES|ORIGIN|PHRASES|•).*$//' | head -c 250 | xargs)

	if ! [[ "$location" =~ ^[A-Za-z0-9_-]+$ ]]; then
	if ! [[ "$location" =~ ^[a-z0-9_-]+$ ]]; then

	local day_of_year=$(date +%j \| sed 's/^0*//')
	local day_of_year=$(date +%j \| sed 's/^0*//')
	[[ -z "$day_of_year" ]] && day_of_year=1

	\| `GOODMORNING_WEATHER_LOCATION` \| City for weather (e.g., San_Francisco, London) \| (none) \| Yes \|
	\| `GOODMORNING_WEATHER_LOCATION` \| City for weather (e.g., San_Francisco, London) \| (none) \| If weather enabled (`GOODMORNING_SHOW_WEATHER=true`) \|

Conversation

bgreg commented Dec 22, 2025

Summary

Changes

Weather (lib/app/display.sh, setup.sh, doctor.sh)

Word of the Day (lib/app/sections/word_of_day.sh)

Section Flag (goodmorning.sh)

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Dec 22, 2025

Uh oh!

Copilot AI commented Dec 22, 2025

Uh oh!

Copilot AI commented Dec 22, 2025

Uh oh!

Copilot AI commented Dec 22, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Weather (`lib/app/display.sh`, `setup.sh`, `doctor.sh`)

Word of the Day (`lib/app/sections/word_of_day.sh`)

Section Flag (`goodmorning.sh`)