Skip to content

Conversation

@yorelog
Copy link

@yorelog yorelog commented Jan 10, 2026

This pull request updates dependencies and makes several improvements to conditional debug logging in the VNC client implementation. The most significant changes are the upgrade of the thiserror and rand crates, a fix to how random numbers are generated for authentication, and the scoping of debug logging variables and code to only compile when the debug-logging feature is enabled.

Dependency upgrades:

  • Updated thiserror from version 1.0 to 2.0 in Cargo.toml for improved error handling support.
  • Updated rand from version 0.8 to 0.9 in Cargo.toml to keep up with the latest random number generation APIs.

Authentication improvements:

  • Changed the random number generator in VncAuth::generate_challenge to use rand::rng() instead of rand::thread_rng() for compatibility with rand 0.9.

Debug logging improvements in VncClient:

  • Scoped debug logging variables such as total_pixels and copy_rect_count to only be compiled when the debug-logging feature is enabled, reducing unnecessary variable usage in release builds. [1] [2]
  • Moved accumulation of debug statistics (like total_pixels and rect_count) into #[cfg(feature = "debug-logging")] blocks to avoid affecting performance or logic when debug logging is disabled. [1] [2] [3]

Copilot AI review requested due to automatic review settings January 10, 2026 12:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates dependencies and improves conditional compilation of debug logging code in the VNC client implementation. The changes aim to reduce code size and eliminate unused variable warnings when the debug-logging feature is disabled.

Changes:

  • Updated thiserror from 1.0 to 2.0 and rand from 0.8 to 0.9 in dependencies
  • Migrated from rand::thread_rng() to rand::rng() for compatibility with rand 0.9
  • Moved debug logging variable declarations and accumulation code inside #[cfg(feature = "debug-logging")] guards

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Cargo.toml Updates dependency versions for thiserror and rand crates
src/auth.rs Changes random number generator initialization to use rand 0.9 API
src/client.rs Scopes debug logging variables to only compile when debug-logging feature is enabled

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1122 to 1129
{
let mut total_pixels = 0u64;
let mut copy_rect_count = 0;
}
#[cfg_attr(
not(feature = "debug-logging"),
allow(unused_variables, unused_assignments)
)]
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

The variables total_pixels and copy_rect_count are declared inside a block that only exists when the debug-logging feature is enabled, but they are used outside this block (at lines 1162-1163, 1232, 1331, 1715, 1722-1723, and 1749). This will cause a compilation error when the feature is enabled because the variables will go out of scope at line 1125.

The variables should be declared at function scope with the #[cfg(feature = "debug-logging")] attribute applied to each variable declaration, not inside a separate block. Remove the braces and apply the attribute to each let statement individually.

Suggested change
{
let mut total_pixels = 0u64;
let mut copy_rect_count = 0;
}
#[cfg_attr(
not(feature = "debug-logging"),
allow(unused_variables, unused_assignments)
)]
let mut total_pixels = 0u64;
#[cfg_attr(
not(feature = "debug-logging"),
allow(unused_variables, unused_assignments)
)]
#[cfg(feature = "debug-logging")]
let mut copy_rect_count = 0;
#[cfg_attr(
not(feature = "debug-logging"),
allow(unused_variables, unused_assignments)
)]

Copilot uses AI. Check for mistakes.
Comment on lines 1126 to 1129
#[cfg_attr(
not(feature = "debug-logging"),
allow(unused_variables, unused_assignments)
)]
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

This #[cfg_attr] attribute is orphaned and not attached to any code statement. It should be removed since the variables it was intended to suppress warnings for (total_pixels and copy_rect_count) are now conditionally compiled with #[cfg(feature = "debug-logging")].

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant