Display the correct SSID for WiFi networks #370
Conversation
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/network.sh (2)
35-35: Cache_get_wifi_ifnameresult to avoid redundant calls.The function is called three times (lines 35, 37, 40). Each call invokes
scutiland multiple pipes. Store the result once and reuse it.Apply this diff:
if [[ $(sw_vers -productVersion) > "25.0" ]]; then - local wifi_network=$(networksetup -listpreferredwirelessnetworks "$(_get_wifi_ifname)" | awk 'NR==2 && sub("\t","") { print; exit }') + local ifname=$(_get_wifi_ifname) + local wifi_network=$(networksetup -listpreferredwirelessnetworks "$ifname" | awk 'NR==2 && sub("\t","") { print; exit }') else - local wifi_network=$(ipconfig getsummary "$(_get_wifi_ifname)" | awk -F ' SSID : ' '/ SSID : / {print $2}') + local ifname=$(_get_wifi_ifname) + local wifi_network=$(ipconfig getsummary "$ifname" | awk -F ' SSID : ' '/ SSID : / {print $2}') fi - local airport=$(networksetup -getairportnetwork "$(_get_wifi_ifname)" | cut -d ':' -f 2 | head -n 1) + local airport=$(networksetup -getairportnetwork "$ifname" | cut -d ':' -f 2 | head -n 1)Also applies to: 37-37, 40-40
42-48: Whitespace handling in airport string comparison.Line 42 checks if
$airportequals the full error message, but aftercutandhead, it may have leading whitespace. The sed on line 43 only trims for the output, not for the condition check. Consider trimming before the comparison for clarity.Apply this diff:
- if [[ $airport != "You are not associated with an AirPort network." ]]; then + airport=$(echo "$airport" | sed 's/^[[:blank:]]*//g') + if [[ $airport != "You are not associated with an AirPort network." ]]; then echo "$wifi_label$airport" | sed 's/^[[:blank:]]*//g'This trims whitespace once before both the check and the output, avoiding the redundant sed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/network.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/network.sh
[error] 34-34: Decimals are not supported. Either use integers only, or use bc or awk to compare.
(SC2072)
[warning] 35-35: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 37-37: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 40-40: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (1)
scripts/network.sh (1)
13-18: Helper function looks good structurally.The fallback logic to find the WiFi interface is sound, though both paths could potentially fail silently without validation.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/network.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/network.sh
[warning] 34-34: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 43-43: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (2)
scripts/network.sh (2)
13-18: Good fallback approach for WiFi interface detection.The function correctly attempts primary detection via
scutiland gracefully falls back to pattern matching if needed. This should handle various macOS versions and configurations well.
37-37: Version comparison correctly uses numeric comparison.The change from string comparison to
bc -lproperly handles multi-part version numbers (e.g.,25.10vs25.2), addressing the previous feedback.
After Darwin 25.0 ipconfig stopped displaying the correct SSID showing <redacted> instead. As a consequence of that the status bar stopped reporting the information. Fix the issue maintaining compatibility with the old versions of MacOS. Signed-off-by: Vincenzo Frascino <[email protected]>
|
Thank you very much for your contribution! |
2 participants
After Darwin 25.0 ipconfig stopped displaying the correct SSID showing instead.
As a consequence of that the status bar stopped reporting the information.
Fix the issue maintaining compatibility with the old versions of MacOS.