feat/add gsm support #350
feat/add gsm support #350
Conversation
📝 WalkthroughWalkthroughAdds GSM support to the Linux network detection script, introducing a GSM label and carrier detection via nmcli. Documentation updates reflect four network states (offline, ethernet, wifi, gsm), revised default display logic, updated Nerdfont labels/icons, and minor formatting adjustments. No changes to non-Linux paths or public signatures. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant network.sh
participant nmcli
Caller->>network.sh: get_ssid (Linux)
network.sh->>network.sh: Load labels (wifi/ethernet/gsm)
alt Check Wi-Fi
network.sh->>nmcli: Query active Wi-Fi SSID
nmcli-->>network.sh: SSID or none
alt SSID found
network.sh-->>Caller: wifi_label + SSID
else No SSID
alt Check GSM
network.sh->>nmcli: Query active GSM connection
nmcli-->>network.sh: Carrier or none
alt GSM active
network.sh-->>Caller: gsm_label + Carrier
else No GSM
network.sh-->>Caller: ethernet_label
end
end
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Theoreticallyhugo
force-pushed
the
feat/network-gsm-support
branch
from
ef5a5fc to
ad154a4
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 (5)
docs/CONFIG.md (4)
618-618: Capitalize and use “by default”; fix sentence flow.Improve readability and grammar.
Apply this diff:
-however, per default **this will only display the ethernet/wifi/gsm you're connected to, if it provides internet access!** +However, by default, **this will only display the ethernet/wifi/gsm you're connected to if it has internet access!**
638-638: Specify a language for the fenced code block (markdownlint MD040).Use a neutral language hint like “text” for icon lists.
Apply this diff:
-``` +```text
632-634: Ensure label examples line up with runtime defaults (trailing space).Your examples include a trailing space (good), but the script’s default for gsm_label is "GSM" without a trailing space, which will render like "GSMVodafone". Either:
- Add a trailing space to the script’s default, or
- Add a small note here that labels should include a trailing space for proper concatenation.
Would you like me to update scripts/network.sh to default to "GSM "?
641-642: Icon grouping note (optional).You combined wifi/gsm icons in one list. Consider keeping separate “wifi” and “gsm” subsections to make it obvious which icon is intended for which label, especially for users scanning quickly.
scripts/network.sh (1)
11-11: Add trailing space to gsm_label default to avoid glued output.Without a trailing space, outputs become “GSMVodafone”. Align with docs (examples include a trailing space).
Apply this diff:
-gsm_label=$(get_tmux_option "@dracula-network-gsm-label" "GSM") +gsm_label=$(get_tmux_option "@dracula-network-gsm-label" "GSM ")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/CONFIG.md(2 hunks)scripts/network.sh(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/network.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🪛 LanguageTool
docs/CONFIG.md
[grammar] ~617-~617: There might be a mistake here.
Context: ...ected, wifi connected, or gsm connected. however, per default **this will only di...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/CONFIG.md
638-638: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| if [ ! -x "$(which nmcli 2> /dev/null)" ];then | ||
| nmcli connection show --active | grep gsm | ||
| gsm_connected=$? | ||
| else | ||
| gsm_connected=false | ||
| fi |
There was a problem hiding this comment.
Broken nmcli presence check (inverted) and unnecessary execution when missing.
The guard is inverted: you run nmcli when it’s not present. Use command -v and set a boolean accordingly.
Apply this diff:
- if [ ! -x "$(which nmcli 2> /dev/null)" ];then
- nmcli connection show --active | grep gsm
- gsm_connected=$?
- else
- gsm_connected=false
- fi
+ if command -v nmcli >/dev/null 2>&1; then
+ if nmcli -t -f TYPE connection show --active | grep -qx 'gsm'; then
+ gsm_connected=true
+ else
+ gsm_connected=false
+ fi
+ else
+ gsm_connected=false
+ fi📝 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.
| if [ ! -x "$(which nmcli 2> /dev/null)" ];then | |
| nmcli connection show --active | grep gsm | |
| gsm_connected=$? | |
| else | |
| gsm_connected=false | |
| fi | |
| if command -v nmcli >/dev/null 2>&1; then | |
| if nmcli -t -f TYPE connection show --active | grep -qx 'gsm'; then | |
| gsm_connected=true | |
| else | |
| gsm_connected=false | |
| fi | |
| else | |
| gsm_connected=false | |
| fi |
🤖 Prompt for AI Agents
In scripts/network.sh around lines 20 to 25, the nmcli presence check is
inverted and runs nmcli when it’s not installed; replace the current guard with
a proper check using command -v nmcli >/dev/null 2>&1, and only run nmcli when
that check succeeds; when nmcli is present run nmcli connection show --active |
grep -q gsm and set gsm_connected to the command exit status ($?), otherwise set
gsm_connected to a non-zero/failure value (e.g., 1 or "false") so you never
execute nmcli when it’s missing.
There was a problem hiding this comment.
i'm not sure if i understand, since i've tried it and it seemed to work. please double check and so will i
There was a problem hiding this comment.
🧩 Analysis chain
@Theoreticallyhugo Let me double-check this logic by testing it directly to make sure I understand it correctly.
🏁 Script executed:
#!/bin/bash
echo "Testing the nmcli presence check logic..."
# First, let's see what happens when nmcli is present
echo "=== Case 1: Testing when nmcli is present ==="
if command -v nmcli >/dev/null 2>&1; then
echo "nmcli is available in this environment"
echo "Testing the current condition: [ ! -x \"\$(which nmcli 2> /dev/null)\" ]"
if [ ! -x "$(which nmcli 2> /dev/null)" ]; then
echo "Condition is TRUE - would execute nmcli command"
else
echo "Condition is FALSE - would set gsm_connected=false"
fi
else
echo "nmcli is NOT available in this environment"
fi
echo ""
echo "=== Case 2: Simulating when nmcli is NOT present ==="
# Let's simulate the condition when nmcli doesn't exist by using a fake command
echo "Testing condition with non-existent command 'fake_nmcli'"
if [ ! -x "$(which fake_nmcli 2> /dev/null)" ]; then
echo "Condition is TRUE - would try to execute the nmcli command (but nmcli doesn't exist)"
else
echo "Condition is FALSE - would set gsm_connected=false"
fi
echo ""
echo "=== Analysis ==="
echo "The logic appears to be:"
echo "- When nmcli is NOT found: condition is TRUE → try to run nmcli (will fail)"
echo "- When nmcli IS found: condition is FALSE → set gsm_connected=false (don't check GSM)"
echo ""
echo "This means GSM detection only works when nmcli is missing (and fails), never when nmcli is present."
Length of output: 1462
@Theoreticallyhugo I've confirmed my analysis - the logic is indeed inverted. Here's what's happening:
Current behavior:
- When
nmcliis NOT installed: condition is TRUE → tries to runnmclicommands (which fail) - When
nmcliIS installed: condition is FALSE → setsgsm_connected=false(never checks GSM)
This means GSM detection is disabled when nmcli is actually available!
You can verify this on your system:
If nmcli is installed on your system, you'll see "Would set gsm_connected=false", meaning GSM detection never runs. The fact that it "works" for you might be because:
- You're testing on a system without nmcli, OR
- You have nmcli but GSM detection isn't actually running (you might be seeing ethernet/wifi instead)
The fix ensures GSM detection only runs when nmcli is available:
🏁 Script executed:
#!/bin/bash
echo "Test this on your system to see the current behavior:"
echo ""
echo "# Check if nmcli is present"
echo "command -v nmcli"
echo ""
echo "# Test the current condition"
echo 'if [ ! -x "$(which nmcli 2> /dev/null)" ]; then'
echo ' echo "Would run nmcli commands"'
echo 'else'
echo ' echo "Would set gsm_connected=false (no GSM detection)"'
echo 'fi'
Length of output: 650
🏁 Script executed:
if command -v nmcli >/dev/null 2>&1; then
if nmcli -t -f TYPE connection show --active | grep -qx 'gsm'; then
gsm_connected=true
else
gsm_connected=false
fi
else
gsm_connected=false
fi
Length of output: 40
@Theoreticallyhugo The diagnostic confirms the logic issue. Since you mentioned it works for you, I suspect one of these scenarios:
-
You tested on a system without nmcli installed - the inverted logic would attempt GSM detection (though it fails), which might appear to "work"
-
You have nmcli but GSM detection never actually runs - you might be seeing the ethernet label instead of GSM detection
To verify what's happening on your system, please run these commands:
# Check if nmcli is present
command -v nmcli
# Test the current condition
if [ ! -x "$(which nmcli 2> /dev/null)" ]; then
echo "Would run nmcli commands"
else
echo "Would set gsm_connected=false (no GSM detection)"
fiIf you have nmcli installed and see "Would set gsm_connected=false", then GSM detection is currently disabled. The fix ensures GSM detection only runs when nmcli is actually available, which is the intended behavior.
The corrected logic is essential for proper GSM detection when nmcli is present on the system.
🧬 Code Graph Analysis Results
- File: scripts/utils.sh
- Snippet (lines 3-12):
get_tmux_option() {
local option="$1"
local default_value="$2"
local option_value="$(tmux show-option -gqv "$option")"
if [ -z "$option_value" ]; then
echo "$default_value"
else
echo "$option_value"
fi
}
- Snippet (lines 3-12):
| if [ -n "$SSID" ]; then | ||
| echo "$wifi_label$SSID" | ||
| elif $gsm_connected; then | ||
| gsm_carrier=$(nmcli connection show --active | grep gsm | cut -d ' ' -f 1) | ||
| echo "$gsm_label$gsm_carrier" | ||
| else |
There was a problem hiding this comment.
Incorrect boolean test and fragile carrier extraction.
- Using
elif $gsm_connected;attempts to execute “true/false/0/1” as a command. Use a string comparison. - Parsing nmcli output with grep/cut is brittle (spaces in NAME). Use terse, fielded output.
Apply this diff:
- if [ -n "$SSID" ]; then
- echo "$wifi_label$SSID"
- elif $gsm_connected; then
- gsm_carrier=$(nmcli connection show --active | grep gsm | cut -d ' ' -f 1)
- echo "$gsm_label$gsm_carrier"
+ if [ -n "$SSID" ]; then
+ echo "$wifi_label$SSID"
+ elif [ "$gsm_connected" = true ]; then
+ gsm_carrier="$(nmcli -t -f NAME,TYPE connection show --active | awk -F: '$2=="gsm"{print $1; exit}')"
+ echo "$gsm_label$gsm_carrier"📝 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.
| if [ -n "$SSID" ]; then | |
| echo "$wifi_label$SSID" | |
| elif $gsm_connected; then | |
| gsm_carrier=$(nmcli connection show --active | grep gsm | cut -d ' ' -f 1) | |
| echo "$gsm_label$gsm_carrier" | |
| else | |
| if [ -n "$SSID" ]; then | |
| echo "$wifi_label$SSID" | |
| elif [ "$gsm_connected" = true ]; then | |
| gsm_carrier="$(nmcli -t -f NAME,TYPE connection show --active | awk -F: '$2=="gsm"{print $1; exit}')" | |
| echo "$gsm_label$gsm_carrier" | |
| else |
🤖 Prompt for AI Agents
In scripts/network.sh around lines 26 to 31, the branch currently tries to
execute the gsm_connected value and uses brittle parsing of nmcli output; change
the conditional to a proper string comparison (e.g. test the variable value with
[ "$gsm_connected" = "true" ] or similar) and replace the grep/cut pipeline with
nmcli's terse, fielded output to reliably extract the active GSM connection name
(use nmcli -t -f NAME,TYPE connection show --active or equivalent and select the
record with TYPE equal to gsm, then take its NAME).
network.sh now supports gsm modules via nmcli.
when connected via gsm, the gsm label is displayed in combination with the carrier.
the gsm label can be set to something custom:
tested on: