-
Notifications
You must be signed in to change notification settings - Fork 734
Fixes #4050. Rename Command.Select and Selecting to Activate/Activating #4470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: tig <[email protected]>
Co-authored-by: tig <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v2_develop #4470 +/- ##
===========================================
Coverage 77.47% 77.47%
===========================================
Files 386 386
Lines 44674 44674
Branches 6282 6282
===========================================
+ Hits 34611 34612 +1
Misses 8210 8210
+ Partials 1853 1852 -1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Co-authored-by: tig <[email protected]>
Co-authored-by: tig <[email protected]>
|
@copilot you didn't run all unit tests before commiting. Tons are failing. Fix please. |
Co-authored-by: tig <[email protected]>
Fixed in commit b7435ee. The issue was automatic |
|
@copilot You need to update docfx/docs. Esp command.md but the rest too. |
Co-authored-by: tig <[email protected]>
Updated in commit fc3ae05. Updated all docfx/docs files including:
All references to |
This reverts commit 6d82bee.
…ithub.com/gui-cs/Terminal.Gui into copilot/rename-command-select-to-activate
…ithub.com/gui-cs/Terminal.Gui into copilot/rename-command-select-to-activate
There was a problem hiding this 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 successfully renames Command.Select and Selecting APIs to Command.Activate and Activating across the Terminal.Gui codebase to clarify semantics between stateful (CheckBox toggle, ListView selection) and stateless (Button focus, MenuItem navigation) views. The renaming improves code clarity by better representing actions that change state or prepare views for interaction.
Key changes:
- Renamed
Command.Selectenum value toCommand.Activatewith updated XML documentation - Renamed
Selectingevent,OnSelecting(), andRaiseSelecting()methods toActivating,OnActivating(), andRaiseActivating()respectively - Updated all key/mouse bindings from
Command.SelecttoCommand.Activate - Comprehensively updated documentation across 7 DocFx files including command.md, events.md, View.md, navigation.md, migratingfromv1.md, mouse.md, and index.md
- Added new command-diagrams.md file with Mermaid diagrams showing command flow
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
| Terminal.Gui/Input/Command.cs | Renamed Select enum to Activate with updated documentation |
| Terminal.Gui/ViewBase/View.Command.cs | Renamed RaiseSelecting() to RaiseActivating(), OnSelecting() to OnActivating(), and Selecting event to Activating |
| Terminal.Gui/ViewBase/View.Keyboard.cs | Updated default Space key binding to Command.Activate |
| Terminal.Gui/ViewBase/View.Mouse.cs | Updated default mouse click bindings to Command.Activate |
| Terminal.Gui/Views/*.cs (30+ files) | Updated all view implementations to use Activating terminology |
| Tests/**/*.cs (15+ test files) | Renamed test methods and variables to use Activating terminology |
| docfx/docs/*.md (7 files) | Updated all documentation references from Select/Selecting to Activate/Activating |
| docfx/docs/command-diagrams.md | New file with Mermaid diagrams showing command flow with Activate |
| Examples/UICatalog/*.cs (8 files) | Updated example code to use Activating event handlers |
Co-authored-by: Copilot <[email protected]>
…ithub.com/gui-cs/Terminal.Gui into copilot/rename-command-select-to-activate
Renamed the `Selecting` event and `OnSelecting` method to `Activating` and `OnActivating` to better reflect their purpose. Updated all related comments, test method names, variables, and assertions in `View` and `ViewCommandTests` to align with the new terminology. Improved code clarity by using `_` for unused parameters in lambda expressions. Renamed properties like `HandleSelecting` to `HandleActivating` and adjusted naming conventions for consistency (e.g., `OnactivatingCount` to `OnActivatingCount`). These changes enhance readability, maintainability, and terminology consistency across the codebase.
Co-authored-by: Copilot <[email protected]>
…ithub.com/gui-cs/Terminal.Gui into copilot/rename-command-select-to-activate
Fixes
Command.SelectandSelectingEvent toActivate#4050Proposed Changes/Todos
Renamed
Command.SelecttoCommand.ActivateandSelectingevent/APIs toActivatingto clarify semantics across stateful (CheckBox toggle, ListView selection) and stateless (Button focus, MenuItem navigation) views.Core Changes
Select→Activatewith updated documentationSelectingevent →Activating,OnSelecting()→OnActivating(),RaiseSelecting()→RaiseActivating()Command.SelecttoCommand.Activatedocfx/docs/including:command.md- Main command system documentation with updated appendix noting completion of renamingevents.md,View.md,navigation.md,migratingfromv1.md,mouse.md,index.md- All terminology updatedScope
Before:
After:
Note on Propagation
The original issue description mentioned adding automatic event propagation to SuperView, but this has been deferred as it broke existing behavior in 16 tests (ScrollBar and Shortcut views). The PR focuses solely on the renaming refactoring. Event propagation can be implemented in a future PR with an opt-in mechanism as originally proposed.
Pull Request checklist:
dotnet testbefore commit///style comments)Original prompt
This section details on the original issue you should resolve
<issue_title>Rename/Refactor
Command.SelectandSelectingEvent toActivate</issue_title><issue_description>## Summary
The
Command.Select,Selectingevent, and related APIs (OnSelecting,RaiseSelecting) should be renamed toCommand.Activate,Activatingevent,OnActivating, andRaiseActivatingto improve clarity and consistency in theCommandsystem. The term "select" is ambiguous for stateless views (e.g.,Button,MenuItemv2, where it only sets focus) and less precise for non-list-based state changes (e.g., toggling aCheckBox). "Activate" better captures the action of changing a view’s state or preparing it for interaction, aligning with both stateful (e.g.,CheckBox,FlagSelector,ListView) and stateless (e.g.,Button,MenuItem) use cases while clearly distinguishing fromAccepting.Additionally, a targeted propagation model for
Command.Activateis needed to support hierarchical components likeMenuBarv2, which requiresActivatingevents from subviews (e.g.,MenuItem) to managePopoverMenuvisibility. To maintain decoupling, subviews should not specify propagation behavior. Instead, superviews should opt-in to receive propagatedActivatingevents via a newPropagateActivatingproperty inView, allowingMenuBarto handle subview activations without subviews knowing superview details.Motivation
The current
Command.SelectandSelectingterminology has several issues:ButtonorMenuItem,Command.Selectonly sets focus, which isn’t typically considered “selection,” leading to developer confusion.CheckBox(toggling state) orFlagSelector(toggling flags), “select” doesn’t fully capture the state change, unlike list-based selection inListView.ListVieworMenu,SelectingandAcceptingcan feel similar (e.g., Enter triggering both), requiring clearer distinction.MenuBarrequiresActivatingevents from subviews to managePopoverMenuvisibility, but the current local handling model relies on view-specific events (e.g.,SelectedMenuItemChanged), which isn’t generalizable. Subviews should remain decoupled from superviews, avoiding knowledge of superview implementation details (e.g.,MenuBar’s popover logic).The
Activateterminology addresses these by:CheckBox, selecting aListViewitem) and preparatory actions (e.g., focusing aButton, navigating aMenuItemv2).Accepting, which confirms actions (e.g., executing a menu command, submitting a dialog).PropagateActivating, enabling superviews likeMenuBarto opt-in to subviewActivatingevents without coupling subviews to superview details.Proposed Changes
1. Rename
Command.Selectand Related APIs toActivateCommand.SelecttoCommand.Activatein theCommandenum.Selectingevent toActivating,OnSelectingtoOnActivating, andRaiseSelectingtoRaiseActivatingin theViewclass and all derived classes (e.g.,Menuv2,MenuItemv2,CheckBox,FlagSelector).SetupCommandsinViewto useCommand.Activate.Shortcut.DispatchCommandand other command handlers to referenceActivating.2. Introduce Targeted Propagation Model with
PropagateActivatingbool PropagateActivatingtoViewto allow superviews (e.g.,MenuBar) to opt-in to receivingActivatingevents from subviews, defaulting tofalsefor local handling.RaiseActivating:SuperView.PropagateActivatingistrueandargs.Handledisfalse, propagate to the superview.MenuBarvfor menu hierarchies.MenuBar:Command.SelectandSelectingEvent toActivate#4050💡 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.