Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Fixed

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

## [1.1.0] - 2025-01-01

### Added
Expand Down
46 changes: 46 additions & 0 deletions src/apps/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ use crate::apps::handlers::flags::Flags;
use crate::utils::connect::connect_to_saved_network;
use crate::utils::disconnect_connection::disconnect_connected_network;
use crate::utils::scan::scan_networks;
use crossterm::cursor;
use crossterm::cursor::DisableBlinking;
use crossterm::cursor::EnableBlinking;
use crossterm::execute;
use ratatui::Frame;
use ratatui::layout::Position;
use std::io;
use std::sync::{Arc, Mutex};
mod delete_handler;
mod help_handlers;
Expand Down Expand Up @@ -53,10 +59,50 @@ impl Default for App {
}

impl App {
/// Render the application UI and manage terminal cursor visibility and position for SSID/password popups.
///
/// When an SSID or password popup is visible, this sets the terminal cursor to the popup's input position
/// and enables cursor blinking; otherwise it hides the cursor and disables blinking.
///
/// # Examples
///
/// ```no_run
/// // Assuming `app` is an initialized `App` and `frame` is a mutable `ratatui::Frame`:
/// // app.draw(&mut frame);
/// ```
fn draw(&self, frame: &mut Frame) {
frame.render_widget(self, frame.area());

// Set cursor position for SSID or Password popups
if self.wifi_credentials.flags.show_ssid_popup
|| self.wifi_credentials.flags.show_password_popup
{
frame.set_cursor_position(Position::new(
frame.area().x + frame.area().width / 4 + self.wifi_credentials.cursor_pos + 1,
frame.area().y + frame.area().height / 4 + 1,
));
Comment on lines +80 to +83

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 | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for cursor_pos assignments and modifications in the codebase
rg -nP --type=rust 'cursor_pos\s*[=+\-]' -B3 -A3

Repository: santoshxshrestha/nmtui

Length of output: 2774


🏁 Script executed:

#!/bin/bash
# Find the popup rendering code in core.rs
rg -nP --type=rust 'width\s*/\s*2' src/apps/core.rs -B5 -A5

Repository: santoshxshrestha/nmtui

Length of output: 48


🏁 Script executed:

#!/bin/bash
# Search for popup rendering code
rg -nP --type=rust 'Block|Paragraph|show_ssid_popup|show_password_popup' src/apps/core.rs -B2 -A2 | head -80

Repository: santoshxshrestha/nmtui

Length of output: 1267


🏁 Script executed:

#!/bin/bash
# Search for render functions related to SSID/password
rg -nP --type=rust 'render.*ssid|render.*password|fn.*ssid|fn.*password' src/apps/core.rs -B2 -A8 | head -100

Repository: santoshxshrestha/nmtui

Length of output: 48


🏁 Script executed:

#!/bin/bash
# Get the size of core.rs and read relevant sections
wc -l src/apps/core.rs
echo "---"
# Look for widget/block creation around the cursor position code
sed -n '50,150p' src/apps/core.rs | cat -n

Repository: santoshxshrestha/nmtui

Length of output: 5497


🏁 Script executed:

#!/bin/bash
# Read the entire core.rs file
cat -n src/apps/core.rs

Repository: santoshxshrestha/nmtui

Length of output: 7739


🏁 Script executed:

#!/bin/bash
# Find widget implementation
fd widget src/apps/ -type f
echo "---"
# If widget.rs exists, show its size
if [ -f src/apps/widget.rs ]; then
  wc -l src/apps/widget.rs
fi
if [ -d src/apps/widget ]; then
  ls -la src/apps/widget/
fi

Repository: santoshxshrestha/nmtui

Length of output: 257


🏁 Script executed:

#!/bin/bash
# Find widget implementation
find src/apps -name "widget*" -o -name "Widget*"
echo "---"
# Look for widget in imports or module declarations
rg -n "mod widget" src/apps/

Repository: santoshxshrestha/nmtui

Length of output: 124


🏁 Script executed:

#!/bin/bash
# Check size and read widget.rs
wc -l src/apps/core/widget.rs
echo "---"
cat -n src/apps/core/widget.rs

Repository: santoshxshrestha/nmtui

Length of output: 13033


Bound cursor position to popup width, not just input string length.

The cursor position calculation (frame.area().x + frame.area().width / 4 + self.wifi_credentials.cursor_pos + 1) has no constraint to keep it within the popup boundaries. While cursor_pos is bounded to the input string's character count in the handlers, it is not bounded to the popup width (area.width / 2). If an input string is very long (longer than the popup can display), the cursor would render outside the visible popup area.

Update the move_cursor_right function in src/apps/handlers/utils.rs to also cap cursor_pos to the popup width (accounting for borders and padding).

🤖 Prompt for AI Agents
In src/apps/core.rs around lines 69-72 the cursor is positioned using
self.wifi_credentials.cursor_pos without constraining it to the popup width;
update src/apps/handlers/utils.rs (move_cursor_right) to clamp cursor_pos to the
visible input area width of the popup: compute popup_width = frame.area().width
/ 2 (as used for the popup), subtract any borders/padding (e.g. 2 for borders
and any left padding) to get visible_input_width, then set cursor_pos =
min(cursor_pos, visible_input_width.saturating_sub(1)) when moving right so the
cursor can never be placed beyond the popup's drawable area; ensure the same
clamping logic is applied wherever cursor_pos is modified.

// The reason for using io::stdout() here is that the crossterm execute! macro needs a writable output target to send
// terminal commands to. io::stdout() provides a handle to the standard output (the terminal), so the commands (Show, MoveTo, EnableBlinking)
// are sent to the terminal for immediate effect.
//
// If you remove io::stdout(), execute! won’t know where to send the commands, resulting in a compilation error.
// You must provide a valid output stream (like io::stdout()) for terminal control commands to work.
// EnableBlinking
let _ = execute!(io::stdout(), cursor::Show, EnableBlinking);
} else {
let _ = execute!(io::stdout(), cursor::Hide, DisableBlinking);
}
Comment on lines +76 to +94

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 | 🟠 Major

Silently ignoring terminal control errors could cause UX issues.

The let _ = execute!(...) pattern on lines 80 and 82 silently discards any errors from cursor visibility/blinking commands. If these operations fail, the cursor could be left in an inconsistent state, degrading the user experience.

Consider logging errors or handling them gracefully to ensure cursor state problems are detectable.

🔎 Proposed fix to handle errors
-            let _ = execute!(io::stdout(), cursor::Show, EnableBlinking);
+            if let Err(e) = execute!(io::stdout(), cursor::Show, EnableBlinking) {
+                eprintln!("Failed to show cursor: {}", e);
+            }
         } else {
-            let _ = execute!(io::stdout(), cursor::Hide, DisableBlinking);
+            if let Err(e) = execute!(io::stdout(), cursor::Hide, DisableBlinking) {
+                eprintln!("Failed to hide cursor: {}", e);
+            }
         }
📝 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
// Set cursor position for SSID or Password popups
if self.wifi_credentials.flags.show_ssid_popup
|| self.wifi_credentials.flags.show_password_popup
{
frame.set_cursor_position(Position::new(
frame.area().x + frame.area().width / 4 + self.wifi_credentials.cursor_pos + 1,
frame.area().y + frame.area().height / 4 + 1,
));
// The reason for using io::stdout() here is that the crossterm execute! macro needs a writable output target to send
// terminal commands to. io::stdout() provides a handle to the standard output (the terminal), so the commands (Show, MoveTo, EnableBlinking)
// are sent to the terminal for immediate effect.
//
// If you remove io::stdout(), execute! won’t know where to send the commands, resulting in a compilation error.
// You must provide a valid output stream (like io::stdout()) for terminal control commands to work.
// EnableBlinking
let _ = execute!(io::stdout(), cursor::Show, EnableBlinking);
} else {
let _ = execute!(io::stdout(), cursor::Hide, DisableBlinking);
}
// Set cursor position for SSID or Password popups
if self.wifi_credentials.flags.show_ssid_popup
|| self.wifi_credentials.flags.show_password_popup
{
frame.set_cursor_position(Position::new(
frame.area().x + frame.area().width / 4 + self.wifi_credentials.cursor_pos + 1,
frame.area().y + frame.area().height / 4 + 1,
));
// The reason for using io::stdout() here is that the crossterm execute! macro needs a writable output target to send
// terminal commands to. io::stdout() provides a handle to the standard output (the terminal), so the commands (Show, MoveTo, EnableBlinking)
// are sent to the terminal for immediate effect.
//
// If you remove io::stdout(), execute! won't know where to send the commands, resulting in a compilation error.
// You must provide a valid output stream (like io::stdout()) for terminal control commands to work.
// EnableBlinking
if let Err(e) = execute!(io::stdout(), cursor::Show, EnableBlinking) {
eprintln!("Failed to show cursor: {}", e);
}
} else {
if let Err(e) = execute!(io::stdout(), cursor::Hide, DisableBlinking) {
eprintln!("Failed to hide cursor: {}", e);
}
}
🤖 Prompt for AI Agents
In src/apps/core.rs around lines 65 to 83, the execute!(io::stdout(), ...) calls
currently use `let _ =` which discards errors; change these to handle the
Result: call execute! and match or use if let Err(e) = ... to log the error
(using the existing logger) and attempt a safe fallback to restore cursor state
(e.g., try the opposite command or ensure cursor is hidden/shown on error).
Ensure both the Show/EnableBlinking and Hide/DisableBlinking paths handle Err by
logging the error with context and performing a best-effort cleanup to keep
terminal cursor state consistent.

}

/// Mark the application to exit by setting its internal exit flag.
///
/// # Examples
///
/// ```
/// let mut app = App::default();
/// app.exit();
/// assert!(app.app_state.exit);
/// ```
fn exit(&mut self) {
self.app_state.exit = true;
}
Expand Down
46 changes: 7 additions & 39 deletions src/apps/core/widget.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
use super::App;

use crossterm::cursor::DisableBlinking;
use crossterm::cursor::EnableBlinking;
use crossterm::cursor::{self, MoveTo};
use crossterm::execute;
use ratatui::widgets::Clear;
use ratatui::{
buffer::Buffer,
Expand All @@ -12,14 +8,13 @@ use ratatui::{
text::Line,
widgets::{Block, Paragraph, Row, Table, TableState, Widget},
};
use std::io;

const INFO_TEXT: [&str; 2] = [
"[Esc] quit | (Ctrl+R) scan for networks | (h) help ",
"(Enter) connect to network | (↑) move up | (↓) move down",
];

const HELP_TEST: [&str; 11] = [
const HELP_TEXT: [&str; 12] = [
"[Esc] quit",
"(Ctrl+c) force quit",
"(Ctrl+R) scan for networks",
Expand All @@ -31,15 +26,15 @@ const HELP_TEST: [&str; 11] = [
"(h) help",
"(?) help",
"(s) view saved networks",
"(x) disconnect from current network",
];

impl Widget for &App {
/// Render the application's terminal UI into the provided drawing area buffer.
/// Render the application's terminal UI into the provided drawing area.
///
/// This draws the main network table and, depending on internal flags and state,
/// overlays the saved-connections list, help menu, delete-confirmation popup,
/// hidden-SSID input popup, password input popup, and status popup. Cursor
/// visibility and blinking are adjusted as needed for input popups.
/// Draws the main network table and, depending on the app's flags and state,
/// conditionally overlays the saved-connections list, help menu, delete-confirmation
/// popup, hidden-SSID input popup, password input popup, and status popup.
///
/// # Examples
///
Expand Down Expand Up @@ -114,7 +109,6 @@ impl Widget for &App {
// handle the render of the saved connections list
if self.flags.show_saved {
Clear.render(area, buf);
let _ = execute!(io::stdout(), cursor::Hide, DisableBlinking);
let saved_block = Block::default()
.title("Saved Connections")
.borders(ratatui::widgets::Borders::ALL)
Expand Down Expand Up @@ -177,12 +171,10 @@ impl Widget for &App {
height: area.height * 2 / 3,
};

let help_paragraph = Paragraph::new(HELP_TEST.join("\n"))
let help_paragraph = Paragraph::new(HELP_TEXT.join("\n"))
.block(help_block)
.style(ratatui::style::Style::default().fg(ratatui::style::Color::White));

let _ = execute!(io::stdout(), cursor::Hide, DisableBlinking);

help_paragraph.render(help_area, buf);
}

Expand All @@ -208,8 +200,6 @@ impl Widget for &App {
.block(popup_block)
.style(ratatui::style::Style::default().fg(ratatui::style::Color::White));

let _ = execute!(io::stdout(), cursor::Hide, DisableBlinking);

confirmation_paragraph.render(popup_area, buf);
}

Expand All @@ -233,16 +223,6 @@ impl Widget for &App {
.block(popup_block)
.style(ratatui::style::Style::default().fg(ratatui::style::Color::White));

let _ = execute!(
io::stdout(),
cursor::Show,
MoveTo(
popup_area.x + self.wifi_credentials.cursor_pos + 1,
popup_area.y + 1,
),
EnableBlinking
);

ssid_paragraph.render(popup_area, buf);
}

Expand All @@ -267,16 +247,6 @@ impl Widget for &App {
.block(popup_block)
.style(ratatui::style::Style::default().fg(ratatui::style::Color::White));

let _ = execute!(
io::stdout(),
cursor::Show,
MoveTo(
popup_area.x + self.wifi_credentials.cursor_pos + 1,
popup_area.y + 1,
),
EnableBlinking
);

password_paragraph.render(popup_area, buf);
}

Expand Down Expand Up @@ -305,8 +275,6 @@ impl Widget for &App {
.block(status_block)
.style(ratatui::style::Style::default().fg(ratatui::style::Color::White));

let _ = execute!(io::stdout(), cursor::Hide, DisableBlinking);

status_paragraph.render(status_area, buf);
}
}
Expand Down
Loading