Skip to content

FIX: weather incorrect param usage #380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
giuse17ita wants to merge 1 commit into
base: master
Choose a base branch
from
Open

FIX: weather incorrect param usage #380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/dracula.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ main() {

elif [ $plugin = "weather" ]; then
IFS=' ' read -r -a colors <<< $(get_tmux_option "@dracula-weather-colors" "orange dark_gray")
script="#($current_dir/weather_wrapper.sh $show_fahrenheit $show_location '$fixed_location' $weather_hide_errors)"
script="#($current_dir/weather_wrapper.sh $show_fahrenheit $show_location '"$fixed_location"' $weather_hide_errors)"
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Shellcheck SC2027: Confusing quoting pattern — $fixed_location is technically unquoted.

The syntax '"$fixed_location"' causes the inner " to close the outer double-quoted string, leaving $fixed_location unquoted during expansion. While this works in assignment context (no word splitting), it's fragile and can misbehave if the location contains special characters like $ or backticks.

Use escaped double quotes instead for clarity and safety:

Suggested fix 
-      script="#($current_dir/weather_wrapper.sh $show_fahrenheit $show_location '"$fixed_location"' $weather_hide_errors)"
+      script="#($current_dir/weather_wrapper.sh $show_fahrenheit $show_location \"$fixed_location\" $weather_hide_errors)"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
script="#($current_dir/weather_wrapper.sh $show_fahrenheit $show_location '"$fixed_location"' $weather_hide_errors)"
script="#($current_dir/weather_wrapper.sh $show_fahrenheit $show_location \"$fixed_location\" $weather_hide_errors)"
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 317-317: The surrounding quotes actually unquote this. Remove or escape them.

(SC2027)

🤖 Prompt for AI Agents 
In `@scripts/dracula.sh` at line 317, The assignment to variable script builds a
command string using the fragile quoting pattern '"$fixed_location"' which
leaves $fixed_location technically unquoted; update the construction in
scripts/dracula.sh so the fixed_location argument is passed with escaped double
quotes (i.e., use an escaped inner quote around $fixed_location) so the
weather_wrapper.sh invocation (current_dir/weather_wrapper.sh with args
show_fahrenheit, show_location, fixed_location, weather_hide_errors) receives a
properly quoted value and prevents word-splitting or special-character
expansion.

Copy link
Copy Markdown
Author

@giuse17ita giuse17ita Jan 21, 2026

Choose a reason for hiding this comment

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

your fix doesnt work ive already tried,

mine solution '"$fixed_location"' works for cases like “Los Angeles” or other multi-word cities.

It allows variable expansion while preserving spaces in the argument,
could be an issue if user pass special characters

but no special characters are used in this context.

Copy link
Copy Markdown
Collaborator

@Theoreticallyhugo Theoreticallyhugo Jan 24, 2026

Choose a reason for hiding this comment

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

since shellcheck actually emits that warning, i'd prefer this to be changed or tested a little more.
i'll try to look into it myself, but that'll take time. so, if you find a solution, I'd be even happier.

In scripts/dracula.sh line 317:
      script="#($current_dir/weather_wrapper.sh $show_fahrenheit $show_location '"$fixed_location"' $weather_hide_errors)"
                                                                                  ^-------------^ SC2027 (warning): The surrounding quotes actually unquote this. Remove or escape them.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 24, 2026

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


elif [ $plugin = "time" ]; then
IFS=' ' read -r -a colors <<< $(get_tmux_option "@dracula-time-colors" "dark_purple white")
Expand Down
2 changes: 1 addition & 1 deletion scripts/weather_wrapper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function main() {

if (((_now - _last) > INTERVAL)); then
# Run weather script here
"${_current_dir}/weather.sh" "$_show_fahrenheit" "$_show_location" "$_location" "$_hide_errors" >"${DATAFILE}"
"${_current_dir}/weather.sh" $_show_fahrenheit $_show_location "$_location" $_hide_errors >"${DATAFILE}"
printf '%s' "$_now" >"${LAST_EXEC_FILE}"
fi

Expand Down