Skip to content

fix(client): address PR #1115 review feedback#1127

Draft
Copilot wants to merge 2 commits intordp-filefrom
copilot/sub-pr-1115
Draft

fix(client): address PR #1115 review feedback#1127
Copilot wants to merge 2 commits intordp-filefrom
copilot/sub-pr-1115

Conversation

Copy link

Copilot AI commented Feb 26, 2026

Applies review feedback on #1115 — the .rdp merge/startup sizing PR.

Changes

  • xtask/src/cov.rs: Revert --no-cfg-coverage additions (unintended change)

  • rdp.rs:295: Restore debug!(?connection_result) — the PR had replaced it with a plain string, losing structured debug output of the connection result

  • config.rs: Remove all debug! calls from Config::parse_from() and its helpers — the logger is not initialized at parse time (parse_from runs before setup_logging() in main()), making every call a silent no-op. Also removes the now-unnecessary SUPPORTED_RDP_PROPERTIES, SENSITIVE_RDP_PROPERTIES, redacted_rdp_line, and related helpers that existed solely for this dead logging. Simplifies helper signatures (rdp_u16_property, rdp_u32_property) and audiomode/KDC proxy handling.

  • Tests: Move ironrdp-client/tests/config_rdp_merge.rsironrdp-testsuite-extra/tests/rdp_client_config.rs

    • Drops the misleading _merge suffix
    • Adds RAII TempRdpFile (Drop-based cleanup, safe on panic)
    • Adds desktop dimension parsing tests (desktopwidth/desktopheight/desktopscalefactor) including out-of-range fallback behavior
    • Removes uuid from ironrdp-client regular deps (was only used by the test)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…it logging, move tests

Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve .rdp merge support and startup sizing fix(client): address PR #1115 review feedback Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants