Skip to content

Conversation

@fxliang
Copy link
Contributor

@fxliang fxliang commented May 8, 2025

close #1515 with yaml configuration, not reg

relative issue #1569

@fxliang fxliang requested review from Copilot and lotem May 8, 2025 06:53
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 introduces a new YAML configuration parameter "hide_ime_mode_icon" to control whether the TSF language bar icon is hidden. Key updates include:

  • Updating the Config struct and its serialization in WeaselIPCData.h to include hide_ime_mode_icon.
  • Propagating the new parameter through WeaselTSF classes to conditionally show or hide UI elements.
  • Adding configuration and messaging support for hide_ime_mode_icon in WeaselIPC/Configurator.cpp and RimeWithWeasel.cpp.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
include/WeaselIPCData.h Updated Config struct with hide_ime_mode_icon flag and its serialization.
WeaselTSF/WeaselTSF.h, WeaselTSF.cpp Introduced _config member and utilized the new flag during UI updates.
WeaselTSF/LanguageBar.cpp Adjusted language bar initialization to determine button visibility based on the new flag.
WeaselTSF/EditSession.cpp Reinitialize language bar when hide_ime_mode_icon flag changes.
WeaselIPC/Configurator.cpp Added YAML configuration support for hide_ime_mode_icon.
RimeWithWeasel.cpp Updated messages and configuration retrieval to include hide_ime_mode_icon.
Comments suppressed due to low confidence (2)

WeaselTSF/LanguageBar.cpp:378

  • [nitpick] An inline comment clarifying why GUID_NULL is used when 'hide_ime_mode_icon' is true would help readers quickly understand the intended behavior.
const GUID langBarGuid = _config.hide_ime_mode_icon ? GUID_NULL : GUID_LBI_INPUTMODE;

RimeWithWeasel/RimeWithWeasel.cpp:32

  • [nitpick] The use of a separate static 'hide_ime_mode_icon' variable in this file seems redundant given the presence of a similar flag in the configuration. Consider consolidating the flag usage to improve consistency.
static bool hide_ime_mode_icon = false;

@fxliang fxliang force-pushed the hide_icon branch 2 times, most recently from 786c51d to 3714399 Compare May 16, 2025 03:07
@fxliang fxliang requested a review from Copilot May 16, 2025 03:07
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 introduces a new parameter, hide_ime_mode_icon, in the YAML configuration to control whether the TSF language bar icon should be hidden. Key changes include:

  • Adding the hide_ime_mode_icon boolean member and updating serialization in Config.
  • Adjusting UI initialization and display logic in the TSF and LanguageBar modules.
  • Updating configuration parsing in the IPC and Rime integration code.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
include/WeaselIPCData.h Added hide_ime_mode_icon member to Config and updated reset/serialization logic.
WeaselTSF/WeaselTSF.h & .cpp Introduced _config in TSF, integrated new parameter usage, and added fake_key function.
WeaselTSF/LanguageBar.cpp Modified language bar initialization to conditionally hide the icon.
WeaselTSF/EditSession.cpp Updated language bar re-initialization based on the new configuration flag.
WeaselIPC/Configurator.cpp Added handling for the hide_ime_mode_icon key.
RimeWithWeasel/RimeWithWeasel.cpp Updated message generation and UI style update to incorporate the new parameter.

…determine if to hide TSF language bar icon.
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