Skip to content

Commit 0723238

Browse files
authored
Merge pull request #5298 from gui-cs/copilot/docs-clarify-ing-vs-ed-events
Fixes #3849. Docs: clarify when to use `-ing` vs `-ed` events (Accepting vs Accepted, etc.)
2 parents b022cb5 + bbdcfbc commit 0723238

3 files changed

Lines changed: 97 additions & 0 deletions

File tree

.claude/rules/event-patterns.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,26 @@
11
# Event Patterns
22

3+
## When to Use `-ing` vs `-ed` Events
4+
5+
Terminal.Gui exposes paired events — `Accepting`/`Accepted`, `Activating`/`Activated`, `ValueChanging`/`ValueChanged`, etc.
6+
7+
**Rule:** Use `-ed` (past-tense) for side-effects. Use `-ing` (present-progressive) only when you need to inspect or cancel the in-flight operation.
8+
9+
```csharp
10+
// ✅ Correct — fire-and-forget side-effect
11+
button.Accepted += (_, _) => DoTheThing ();
12+
13+
// ✅ Correct — actually cancels
14+
button.Accepting += (_, e) => { if (!CanProceed ()) e.Handled = true; };
15+
16+
// ❌ Wrong — handler ignores EventArgs; use Accepted instead
17+
button.Accepting += (_, _) => DoTheThing ();
18+
```
19+
20+
If the handler body doesn't reference `e` at all (or ignores `e.Handled`, `e.Cancel`, and the candidate value), it belongs on the `-ed` event.
21+
22+
The `-ing` event runs synchronously in the middle of the dispatch path; subscribing when you don't need to cancel adds unnecessary overhead and misleads readers.
23+
324
## Lambda Parameters
425

526
**Replace unused parameters with discards `_`:**

Terminal.Gui/ViewBase/View.Command.cs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,16 @@ private void SetupCommands ()
499499
/// <para>
500500
/// See <see cref="View.RaiseAccepting"/> for more information.
501501
/// </para>
502+
/// <para>
503+
/// <strong>When to use:</strong> Subscribe to <see cref="Accepting"/> only when you need to inspect or cancel the
504+
/// in-flight Accept operation (e.g., set <c>e.Handled = true</c> to prevent the accept). For simple side-effects
505+
/// that don't need to cancel, subscribe to <see cref="Accepted"/> instead — it is lighter-weight and communicates
506+
/// intent more clearly.
507+
/// </para>
508+
/// <para>
509+
/// <strong>Rule of thumb:</strong> If your handler doesn't read or set anything on <see cref="CommandEventArgs"/>
510+
/// (no <c>e.Handled</c>, no inspection of context), use <see cref="Accepted"/>.
511+
/// </para>
502512
/// </remarks>
503513
public event EventHandler<CommandEventArgs>? Accepting;
504514

@@ -543,6 +553,19 @@ protected virtual void OnAccepted (ICommandContext? ctx) { }
543553
/// <para>
544554
/// See <see cref="RaiseAccepted"/> for more information.
545555
/// </para>
556+
/// <para>
557+
/// <strong>When to use:</strong> Subscribe to <see cref="Accepted"/> for fire-and-forget side-effects — things that
558+
/// happen <em>after</em> the accept has completed and cannot be cancelled. This is the right choice for the vast
559+
/// majority of button-click–style handlers.
560+
/// </para>
561+
/// <para>
562+
/// <strong>Example:</strong>
563+
/// <code>
564+
/// button.Accepted += (_, _) =&gt; DoTheThing (); // correct — side-effect only
565+
/// button.Accepting += (_, e) =&gt; { if (!CanProceed ()) e.Handled = true; }; // correct — cancels
566+
/// button.Accepting += (_, _) =&gt; DoTheThing (); // wrong — use Accepted instead
567+
/// </code>
568+
/// </para>
546569
/// </remarks>
547570
public event EventHandler<CommandEventArgs>? Accepted;
548571

@@ -843,6 +866,18 @@ private void BubbleActivatedUp (ICommandContext? ctx, bool compositeOnly = false
843866
/// Set CommandEventArgs.Handled to <see langword="true"/> to indicate the event was handled and processing should
844867
/// stop.
845868
/// </summary>
869+
/// <remarks>
870+
/// <para>
871+
/// <strong>When to use:</strong> Subscribe to <see cref="Activating"/> only when you need to inspect or cancel the
872+
/// in-flight Activate operation (e.g., set <c>e.Handled = true</c> to prevent the state change). For simple
873+
/// side-effects that don't need to cancel, subscribe to <see cref="Activated"/> instead — it is lighter-weight and
874+
/// communicates intent more clearly.
875+
/// </para>
876+
/// <para>
877+
/// <strong>Rule of thumb:</strong> If your handler doesn't read or set anything on <see cref="CommandEventArgs"/>
878+
/// (no <c>e.Handled</c>, no inspection of context), use <see cref="Activated"/>.
879+
/// </para>
880+
/// </remarks>
846881
public event EventHandler<CommandEventArgs>? Activating;
847882

848883
/// <summary>
@@ -913,6 +948,16 @@ protected virtual void OnActivated (ICommandContext? ctx) { }
913948
/// Event raised when the user has performed an action (e.g. <see cref="Command.Activate"/>) causing the
914949
/// View to change state or preparing it for interaction.
915950
/// </summary>
951+
/// <remarks>
952+
/// <para>
953+
/// Unlike <see cref="Activating"/>, this event cannot be cancelled. It is raised after the View has activated.
954+
/// </para>
955+
/// <para>
956+
/// <strong>When to use:</strong> Subscribe to <see cref="Activated"/> for fire-and-forget side-effects — things
957+
/// that happen <em>after</em> the activation has completed and cannot be cancelled. This is the right choice for
958+
/// the vast majority of state-change–reaction handlers.
959+
/// </para>
960+
/// </remarks>
916961
public event EventHandler<EventArgs<ICommandContext?>>? Activated;
917962

918963
#endregion Activate

docfx/docs/events.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,37 @@ Use this decision tree to choose the right pattern:
4242
| Simple notification (no cancel) | `EventHandler` | [Recipe 3](#recipe-3-simple-notification) |
4343
| Property notification (MVVM) | `INotifyPropertyChanged` | [Recipe 4](#recipe-4-mvvm-property-notification) |
4444

45+
## When to Use `-ing` vs `-ed` Events
46+
47+
Terminal.Gui exposes paired events on many surfaces — `Accepting`/`Accepted`, `Activating`/`Activated`, `ValueChanging`/`ValueChanged`, etc. Use this rule to choose:
48+
49+
> **Use `-ed` (past-tense) events for side-effects. Use `-ing` (present-progressive) events only when you actually need to inspect or cancel the in-flight operation.**
50+
51+
If your handler doesn't read or set anything on the `EventArgs` (no `e.Handled`, no `e.Cancel`, no inspection of the candidate value), you want the `-ed` event. The `-ing` event runs synchronously in the middle of the dispatch path and is heavier for both the framework and the reader of your code.
52+
53+
### Concrete Examples
54+
55+
```csharp
56+
// ✅ Correct — fire-and-forget side-effect belongs on the -ed event
57+
button.Accepted += (_, _) => DoTheThing ();
58+
59+
// ✅ Correct — actually needs to cancel, so -ing is right
60+
button.Accepting += (_, e) => { if (!CanProceed ()) e.Handled = true; };
61+
62+
// ❌ Wrong — handler ignores EventArgs; should use Accepted
63+
button.Accepting += (_, _) => DoTheThing ();
64+
```
65+
66+
The same rule applies to every other paired event in the framework:
67+
68+
| Use `-ed` (side-effect) | Use `-ing` (inspect / cancel) |
69+
|-------------------------|-------------------------------|
70+
| `Accepted` | `Accepting` |
71+
| `Activated` | `Activating` |
72+
| `ValueChanged` | `ValueChanging` |
73+
| `TextChanged` | `TextChanging` |
74+
| `TitleChanged` | `TitleChanging` |
75+
4576
## See Also
4677

4778
* [Cancellable Work Pattern](cancellable-work-pattern.md) - Conceptual overview

0 commit comments

Comments
 (0)