Skip to content
23 changes: 18 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,26 @@ All notable changes to this project will be documented in this file.

### Fixed

- Fixed the rerendering issue of the cursor by removing it from the render method and placing it to the draw method.
### Fixed

- Fixed the re-rendering issue of the cursor by removing it from the render method and placing it to the draw method.
- Fixed the issue of pop-up for pressing 'd' key in the unsaved connection from the main list.
Comment thread
vimlinuz marked this conversation as resolved.

### Changed

- Added a flag to indicate whether the app is scanning the networks or not.
- Changed the type of list that was send to the scan_networks function from main App `struct` to be `RwLock` wrapped type rather than Arc wrapped type.

### Changed

- Changed the type of list that was send to the scan_networks function from main App `struct` to be `RwLock` wrapped type rather than Arc wrapped type.
- Changed the render method to render the scanning status with small animation in accordance with the scanning flag rather then the `.try_lock()` method that was used earlier.
Comment thread
vimlinuz marked this conversation as resolved.

## [1.1.0] - 2025-01-01

### Added

- Listner for disconnecting the current connection
- Listener for disconnecting the current connection
- Helper function to disconnect from the current connection
- Utility function that will actually disconnect from the current connection

Expand All @@ -24,7 +37,7 @@ All notable changes to this project will be documented in this file.

- Fixed the issue in the saved-connection list where when we try to delete a connection, it was doing the deletion from the index of the main list instead of the saved connection list.
- Fixed the refresh issue of the list after the deletion of a saved connection.
- Fixed the outo of bound issue when the list gets refreshed after deletion.
- Fixed the out of bound issue when the list gets refreshed after deletion.

## [1.0.0] - 2025-12-30

Expand All @@ -33,8 +46,8 @@ All notable changes to this project will be documented in this file.
- documentation and comments
- help menu
- list for saved connections,
- delete connection confirmation popup,
- listeners for all the popups/menus
- delete connection confirmation pop-up,
- listeners for all the pop-ups/menus

### Changed

Expand Down
30 changes: 20 additions & 10 deletions src/apps/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@ use crossterm::execute;
use ratatui::Frame;
use ratatui::layout::Position;
use std::io;
use std::sync::{Arc, Mutex};
use std::sync::Arc;
mod delete_handler;
mod help_handlers;
use std::sync::RwLock;
use std::sync::atomic::AtomicBool;

#[derive(Debug)]
pub struct App {
wifi_credentials: WifiInputState,
wifi_list: Arc<Mutex<Vec<WifiNetwork>>>,
wifi_list: Arc<RwLock<Vec<WifiNetwork>>>,
selected: usize,
app_state: AppState,
saved_connection: SavedConnections,
Expand All @@ -45,15 +47,23 @@ impl Default for App {
/// assert!(app.wifi_list.lock().unwrap().is_empty());
/// ```
fn default() -> Self {
let wifi_list = Arc::new(Mutex::new(Vec::new()));
scan_networks(wifi_list.clone());
let wifi_list = Arc::new(RwLock::new(Vec::new()));

// Setting up scanning flag
let is_scanning = Arc::new(AtomicBool::new(true));
scan_networks(wifi_list.clone(), is_scanning.clone());
Self {
wifi_credentials: WifiInputState::default(),
wifi_list,
selected: 0,
app_state: AppState::default(),
saved_connection: SavedConnections::default(),
flags: Flags::default(),
flags: {
Flags {
is_scanning,
..Default::default()
}
},
}
}
}
Expand Down Expand Up @@ -108,7 +118,7 @@ impl App {
}

fn prepare_to_connect(&mut self) {
match self.wifi_list.try_lock() {
match self.wifi_list.write() {
Ok(wifi_list) => {
// if the selected network is already in use, do nothing
if wifi_list[self.selected].in_use {
Expand All @@ -120,7 +130,7 @@ impl App {
self.wifi_credentials.flags.show_status_popup = true;

// refresh the network list after connection attempt
scan_networks(self.wifi_list.clone());
scan_networks(self.wifi_list.clone(), self.flags.is_scanning.clone());
Comment thread
vimlinuz marked this conversation as resolved.
}
// if the selected network is hidden network option
// the show status popup will be handled by the password input listener
Expand All @@ -142,7 +152,7 @@ impl App {
self.wifi_credentials.status = status;
self.wifi_credentials.flags.show_status_popup = true;
// refresh the network list after connection attempt
scan_networks(self.wifi_list.clone());
scan_networks(self.wifi_list.clone(), self.flags.is_scanning.clone());
}
// else show the password popup
else {
Expand All @@ -158,7 +168,7 @@ impl App {
}

fn update_selected_network(&mut self, direction: isize) {
if let Ok(wifi_list) = self.wifi_list.try_lock() {
if let Ok(wifi_list) = self.wifi_list.read() {
let len = wifi_list.len();
if len > 0 {
self.selected =
Expand All @@ -171,7 +181,7 @@ impl App {
fn disconnect(&mut self) {
self.wifi_credentials.status = disconnect_connected_network(self.wifi_list.clone());
self.wifi_credentials.flags.show_status_popup = true;
scan_networks(self.wifi_list.clone());
scan_networks(self.wifi_list.clone(), self.flags.is_scanning.clone());
}

fn reset_selection(&mut self) {
Expand Down
8 changes: 6 additions & 2 deletions src/apps/core/delete_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,13 @@ impl App {
self.saved_connection.fetch_saved_connections();
} else {
// this one will delete the connection from the wifi list
delete_connection(self.wifi_list.lock().unwrap()[self.selected].ssid.clone());
delete_connection(
self.wifi_list.read().expect("WifiNetworks lock poisoned")[self.selected]
.ssid
.clone(),
);
self.reset_selection();
scan_networks(self.wifi_list.clone());
scan_networks(self.wifi_list.clone(), self.flags.is_scanning.clone());
Comment thread
vimlinuz marked this conversation as resolved.
}
self.flags.show_delete_confirmation = false;
}
Expand Down
11 changes: 9 additions & 2 deletions src/apps/core/event_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl App {
kind: Press,
..
}) => {
scan_networks(self.wifi_list.clone());
scan_networks(self.wifi_list.clone(), self.flags.is_scanning.clone());
}
Comment thread
vimlinuz marked this conversation as resolved.
Event::Key(KeyEvent {
code: KeyCode::Enter,
Expand Down Expand Up @@ -115,7 +115,14 @@ impl App {
kind: Press,
..
}) => {
self.flags.show_delete_confirmation = true;
if self
.wifi_list
.read()
.expect("Wifi list lock poisoned while deleting")[self.selected]
.is_saved
{
self.flags.show_delete_confirmation = true;
}
}
Comment thread
vimlinuz marked this conversation as resolved.
Event::Key(KeyEvent {
code: KeyCode::Char('s'),
Expand Down
75 changes: 51 additions & 24 deletions src/apps/core/widget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use ratatui::{
text::Line,
widgets::{Block, Paragraph, Row, Table, TableState, Widget},
};
use std::sync::atomic::Ordering;
use std::time::{self, SystemTime};

const INFO_TEXT: [&str; 2] = [
"[Esc] quit | (Ctrl+R) scan for networks | (h) help ",
Expand Down Expand Up @@ -63,31 +65,56 @@ impl Widget for &App {
);

let mut rows = Vec::new();
match self.wifi_list.try_lock() {
Ok(wifi_list) => {
for (i, network) in wifi_list.iter().enumerate() {
let ssid = if network.in_use {
format!("* {}", network.ssid)
} else {
network.ssid.clone()
};
let mut row = Row::new(vec![
ssid,
network.security.clone(),
network.is_saved.to_string(),
]);
if i == self.selected {
row = row.style(
ratatui::style::Style::default()
.fg(ratatui::style::Color::Black)
.bg(ratatui::style::Color::White),
);
}
rows.push(row);

if self.flags.is_scanning.load(Ordering::SeqCst) {
let spinner = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"];

// Use some time-based value for the index
// this will select a different spinner character every 100ms
// and push it to teh rows vector
// as this function is called every frame during the scanning process
// this will create the illusion of a spinner animation
let index = SystemTime::now()
.duration_since(time::UNIX_EPOCH)
.expect("Time went backwards")
.as_millis()
/ 100;

let spinner_char = spinner[index as usize % spinner.len()];

rows.push(Row::new(vec![
format!("{} Scanning...", spinner_char),
"".into(),
"".into(),
]));
} else {
// This will not panic untill the thread holding the write lock panics so we can just unwrap here
let wifi_list = self.wifi_list.read().expect("WifiNetworks lock poisoned");

// populate the rows of the table that displays the wifi networks and highlights the selected one
// This will never be empty because we always add the "Connect to Hidden network" entry
// untill I implement a better way to handle the addition of hidden networks

// TODO: implement the case if there are not networks found with a message to the user
for (i, network) in wifi_list.iter().enumerate() {
let ssid = if network.in_use {
format!("* {}", network.ssid)
} else {
network.ssid.clone()
};
let mut row = Row::new(vec![
ssid,
network.security.clone(),
network.is_saved.to_string(),
]);
if i == self.selected {
row = row.style(
ratatui::style::Style::default()
.fg(ratatui::style::Color::Black)
.bg(ratatui::style::Color::White),
);
}
}
Err(_) => {
rows.push(Row::new(vec!["Scanning...", ""]));
rows.push(row);
}
}

Expand Down
10 changes: 7 additions & 3 deletions src/apps/handlers/flags.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
use std::sync::Arc;
use std::sync::atomic::AtomicBool;

#[derive(Debug, Default)]
pub struct Flags {
pub is_hidden: bool,
pub is_scanning: Arc<AtomicBool>,
pub show_delete_confirmation: bool,
pub show_help: bool,
pub show_password_popup: bool,
pub show_saved: bool,
pub show_ssid_popup: bool,
pub show_status_popup: bool,
pub show_delete_confirmation: bool,
pub show_saved: bool,
pub show_help: bool,
}
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ struct AppState {

fn main() -> Result<()> {
color_eyre::install()?;
tui().unwrap();
tui().expect("Failed to run TUI application");
Ok(())
}
6 changes: 3 additions & 3 deletions src/utils/disconnect_connection.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::{WifiNetwork, apps::handlers::status::Status};
use std::{
process::{Command, ExitStatus},
sync::{Arc, Mutex},
sync::{Arc, RwLock},
};

pub fn disconnect_connected_network(wifi_list: Arc<Mutex<Vec<WifiNetwork>>>) -> Status {
let list = wifi_list.lock().expect("WifiNetworks lock poisoned");
pub fn disconnect_connected_network(wifi_list: Arc<RwLock<Vec<WifiNetwork>>>) -> Status {
let list = wifi_list.read().expect("WifiNetworks lock poisoned");

for network in list.iter() {
if network.in_use {
Expand Down
10 changes: 7 additions & 3 deletions src/utils/scan.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use crate::WifiNetwork;
use crate::utils::saved_connection::saved_connections;
use std::process::Command;
use std::sync::{Arc, Mutex};
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, RwLock};
use std::thread;

pub fn scan_networks(wifi_list: Arc<Mutex<Vec<WifiNetwork>>>) {
pub fn scan_networks(wifi_list: Arc<RwLock<Vec<WifiNetwork>>>, is_scanning: Arc<AtomicBool>) {
is_scanning.store(true, Ordering::SeqCst);
Comment on lines +8 to +9

@coderabbitai coderabbitai Bot Jan 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Scanning flag can be stuck at true indefinitely.

The is_scanning flag is set to true here but only reset to false at Line 62 in the happy path. If the spawned thread panics (lines 15 or 60) or returns early (Line 18), the flag remains true forever, causing the UI to display a spinner indefinitely.

🔎 Recommended fix using a scope guard pattern
 pub fn scan_networks(wifi_list: Arc<RwLock<Vec<WifiNetwork>>>, is_scanning: Arc<AtomicBool>) {
-    is_scanning.store(true, Ordering::SeqCst);
-    // // nmcli -t -f IN-USE,SSID,SECURITY device wifi list
     thread::spawn(move || {
+        is_scanning.store(true, Ordering::SeqCst);
+        // Ensure the flag is reset when the thread exits (success, error, or panic)
+        let _guard = scopeguard::guard((), |_| {
+            is_scanning.store(false, Ordering::SeqCst);
+        });
+
         let output = Command::new("nmcli")
             .args(["-t", "-f", "IN-USE,SSID,SECURITY", "device", "wifi", "list"])
             .output()
-            .expect("Failed to execute nmcli command");
+            .ok()?;

         if !output.status.success() {
-            return;
+            return None;
         }
         let stdout = String::from_utf8_lossy(&output.stdout);
         let mut networks: Vec<WifiNetwork> = Vec::new();

         let saved_networks = saved_connections();

         for line in stdout.lines() {
             let mut parts = line.splitn(3, ':');

             let in_use = parts.next() == Some("*");
             let ssid = parts.next().unwrap_or("").to_string();

             let security = parts
                 .next()
                 .map(|s| {
                     if s.is_empty() {
                         "Unsecured".to_string()
                     } else {
                         s.to_string()
                     }
                 })
                 .unwrap_or("?".to_string());

             let is_saved = saved_networks.contains(&ssid);

             if !ssid.is_empty() {
                 networks.push(WifiNetwork {
                     is_saved,
                     in_use,
                     ssid,
                     security,
                 })
             }
         }
         networks.push(WifiNetwork {
             is_saved: false,
             in_use: false,
             ssid: "Connect to Hidden network".to_string(),
             security: "?".to_string(),
         });

-        let mut wifi_list_lock = wifi_list.write().expect("WifiNetworks lock poisoned");
+        let mut wifi_list_lock = wifi_list.write().ok()?;
         *wifi_list_lock = networks;
-        is_scanning.store(false, Ordering::SeqCst);
+        Some(())
     });
 }

Note: This requires adding the scopeguard crate. Alternatively, you can use a custom Drop implementation or wrap the thread body in a closure that always resets the flag.

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

can you create issue related to that I will fix it later

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Create a GitHub issue for the scanning flag bug
gh issue create \
  --title "Critical: is_scanning flag can remain stuck at true indefinitely" \
  --body "## Description

The \`is_scanning\` flag in \`src/utils/scan.rs\` can become permanently stuck at \`true\`, causing the UI to display a spinner indefinitely.

## Problem

In the \`scan_networks\` function, the \`is_scanning\` flag is set to \`true\` at the start of the function, but it is only reset to \`false\` at the end of the spawned thread's happy path. If the thread panics (e.g., at the \`expect()\` calls) or returns early (e.g., when the command fails), the flag remains \`true\` forever.

## Impact

When this occurs, users will see a continuous scanning animation and will be unable to interact with the network list, effectively breaking the application's functionality.

## Related PR and Comment

- PR: https://github.com/santoshxshrestha/nmtui/pull/30
- Original comment: https://github.com/santoshxshrestha/nmtui/pull/30#discussion_r2659084183
- Reported by: @coderabbitai" \
  --assignee santoshxshrestha

Length of output: 51


@santoshxshrestha, I've created issue #31 to track this bug. The issue describes the problem where the is_scanning flag can get stuck at true indefinitely, along with the impact on users. You can address it in a future PR. 👍


🧬 Code Graph Analysis Results

[src/utils/saved_connection.rs] lines 3-28

pub fn saved_connections() -> Vec<String> {
    // nmcli -t -f NAME,TYPE connection show
    let output = Command::new("nmcli")
        .args(["-t", "-f", "NAME,TYPE", "connection", "show"])
        .output()
        .expect(" Failed to execute nmcli command");

    let mut ssids: Vec<String> = Vec::new();
    if !output.status.success() {
        return ssids;
    }
    let stdout = String::from_utf8_lossy(&output.stdout);

    // header is already handled by nmcli with -t flag
    for line in stdout.lines() {
        let mut parts = line.splitn(2, ':');
        let ssid = parts.next().unwrap_or("").to_string();
        let connection_type = parts.next().unwrap_or("").to_string();

        // we are only interested in wifi saved connections
        if !ssid.is_empty() && !connection_type.is_empty() && connection_type == "802-11-wireless" {
            ssids.push(ssid);
        }
    }
    ssids
}

[src/utils/scan.rs] (function: scan_networks)

  • Summary: scan_networks takes an Arc<RwLock<Vec>> and an Arc is_scanning. It marks scanning as true, spawns a thread to execute the system command nmcli to list wifi networks, parses each line of output (IN-USE:SSID:SECURITY) into WifiNetwork entries (computing in_use, ssid, security, and is_saved by checking saved_connections()). It adds a synthetic "Connect to Hidden network" entry, writes the resulting vector into wifi_list, and then marks is_scanning as false. The function returns immediately after spawning the thread.

// // nmcli -t -f IN-USE,SSID,SECURITY device wifi list
thread::spawn(move || {
let mut wifi_list_lock = wifi_list.lock().unwrap();
let output = Command::new("nmcli")
.args(["-t", "-f", "IN-USE,SSID,SECURITY", "device", "wifi", "list"])
.output()
Expand Down Expand Up @@ -55,6 +56,9 @@ pub fn scan_networks(wifi_list: Arc<Mutex<Vec<WifiNetwork>>>) {
ssid: "Connect to Hidden network".to_string(),
security: "?".to_string(),
});

let mut wifi_list_lock = wifi_list.write().expect("WifiNetworks lock poisoned");
Comment thread
vimlinuz marked this conversation as resolved.
*wifi_list_lock = networks;
is_scanning.store(false, Ordering::SeqCst);
});
}
2 changes: 1 addition & 1 deletion src/utils/tui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ use crate::apps::core::App;
pub fn tui() -> Result<(), Box<dyn std::error::Error>> {
let mut terminal = ratatui::init();
let app_result = App::default().run(&mut terminal);
ratatui::try_restore().unwrap();
ratatui::try_restore().expect("Failed to restore terminal");
app_result
}
Loading