Terminal.Gui/docs/analysis/cwp_helper_migration_analysis.md

CWP Helper Class Migration Analysis

Executive Summary

This document provides a detailed analysis of the effort required to replace all direct CWP implementations with CWP helper classes (CWPWorkflowHelper, CWPPropertyHelper, CWPEventHelper).

Current State

• CWP Helper Classes: 3 (CWPWorkflowHelper, CWPPropertyHelper, CWPEventHelper)
• Current Helper Usage: 2 files (View.Drawing.Scheme.cs only)
• Direct CWP Implementations: 28 methods across 11 files
• Total CWP Patterns: 31 instances

Migration Summary

• Can Use CWPWorkflowHelper: 24 instances (77%)
• Can Use CWPPropertyHelper: 2 instances (6%)
• Cannot Use Helpers (Custom Logic): 5 instances (16%)
• Estimated Total Effort: 3-4 weeks
• Risk Level: MEDIUM

CWP Helper Classes Overview

1. CWPWorkflowHelper

Purpose: Single-phase workflows with optional default action

Signature:

public static bool? Execute<T>(
    Func<ResultEventArgs<T>, bool> onMethod,
    EventHandler<ResultEventArgs<T>>? eventHandler,
    ResultEventArgs<T> args,
    Action? defaultAction = null)

Use When:

• Simple workflow with virtual method + event
• Returns bool? (true=handled, false=not handled, null=no subscribers)
• Optional default action if not handled
• No complex state management between phases

2. CWPPropertyHelper

Purpose: Property change with pre/post notifications

Signature:

public static bool ChangeProperty<T>(
    T currentValue,
    T newValue,
    Func<ValueChangingEventArgs<T>, bool> onChanging,
    EventHandler<ValueChangingEventArgs<T>>? changingEvent,
    Action<ValueChangedEventArgs<T>>? onChanged,
    EventHandler<ValueChangedEventArgs<T>>? changedEvent,
    out T finalValue)

Use When:

• Property setter with validation
• Need both "changing" (cancellable) and "changed" (notification) events
• Two-phase pattern (pre-change, post-change)

3. CWPEventHelper

Purpose: Event-only pattern (no virtual method)

Signature:

public static bool Execute<T>(
    EventHandler<ResultEventArgs<T>>? eventHandler,
    ResultEventArgs<T> args)

Use When:

• Only have event, no virtual method
• Simple event invocation with handled check
• Rarely used in current codebase

Detailed Migration Analysis

Category 1: Mouse Events (HIGH PRIORITY)

1.1 OnMouseEvent / MouseEvent

File: Terminal.Gui/ViewBase/View.Mouse.cs:315-338

Current Implementation:

public bool RaiseMouseEvent (MouseEventArgs mouseEvent)
{
    // Pre-processing: MouseHeldDown logic
    if (WantContinuousButtonPressed && MouseHeldDown != null)
    {
        if (mouseEvent.IsPressed)
        {
            MouseHeldDown.Start ();
        }
        else
        {
            MouseHeldDown.Stop ();
        }
    }

    // CWP pattern
    if (OnMouseEvent (mouseEvent) || mouseEvent.Handled)
    {
        return true;
    }

    MouseEvent?.Invoke (this, mouseEvent);

    return mouseEvent.Handled;
}

Can Use Helper?: ❌ NO - Custom pre-processing logic

Reason: The MouseHeldDown logic must execute before the CWP pattern. CWPWorkflowHelper doesn't support pre-processing code before the workflow.

Migration Difficulty: N/A (Cannot migrate)

Recommendation: Keep as-is. This is appropriate custom logic.

---

1.2 OnMouseClick / MouseClick

File: Terminal.Gui/ViewBase/View.Mouse.cs:470-498

Current Implementation:

protected bool RaiseMouseClickEvent (MouseEventArgs args)
{
    // Pre-conditions
    if (!Enabled)
    {
        return args.Handled = false;
    }

    // CWP pattern
    if (OnMouseClick (args) || args.Handled)
    {
        return args.Handled;
    }

    MouseClick?.Invoke (this, args);

    if (args.Handled)
    {
        return true;
    }

    // Post-conditions: Invoke commands
    args.Handled = InvokeCommandsBoundToMouse (args) == true;

    return args.Handled;
}

Can Use Helper?: ⚠️ PARTIAL - Pre/post conditions complicate

Proposed Migration:

protected bool RaiseMouseClickEvent (MouseEventArgs args)
{
    // Pre-condition check stays outside
    if (!Enabled)
    {
        return args.Handled = false;
    }

    // Use helper but wrap return value
    bool? result = CWPWorkflowHelper.Execute(
        onMethod: _ => OnMouseClick(args),
        eventHandler: MouseClick,
        args: new ResultEventArgs<MouseEventArgs> { Result = args },
        defaultAction: null);

    // Post-condition: Invoke commands if not handled
    if (result != true && !args.Handled)
    {
        args.Handled = InvokeCommandsBoundToMouse(args) == true;
    }

    return args.Handled;
}

Migration Difficulty: ⭐⭐⭐ HARD

Issues:

1. MouseEventArgs doesn't fit ResultEventArgs pattern cleanly
2. Pre/post conditions require wrapper logic
3. Command invocation logic stays outside helper
4. More complex than direct implementation

Recommendation: ❌ Don't migrate. Direct implementation is clearer.

---

1.3 OnMouseWheel / MouseWheel

File: Terminal.Gui/ViewBase/View.Mouse.cs:574-600

Analysis: Same pattern as OnMouseClick

Can Use Helper?: ⚠️ PARTIAL (same issues)

Migration Difficulty: ⭐⭐⭐ HARD

Recommendation: ❌ Don't migrate

---

1.4 OnMouseEnter / MouseEnter

File: Terminal.Gui/ViewBase/View.Mouse.cs:69-98

Current Implementation:

internal bool? NewMouseEnterEvent (CancelEventArgs eventArgs)
{
    if (!CanBeVisible (this))
    {
        return null;
    }

    if (OnMouseEnter (eventArgs))
    {
        return true;
    }

    MouseEnter?.Invoke (this, eventArgs);

    if (eventArgs.Cancel)
    {
        return true;
    }

    // Post-processing: Set mouse state
    MouseState |= MouseState.In;

    if (HighlightStates != MouseState.None)
    {
        SetNeedsDraw ();
    }

    return false;
}

Can Use Helper?: ❌ NO - Post-processing state management

Reason: Setting MouseState and SetNeedsDraw after event is essential logic that can't be a "default action"

Migration Difficulty: N/A

Recommendation: ❌ Don't migrate

---

Category 2: Keyboard Events (HIGH PRIORITY)

2.1 OnKeyDown / KeyDown

File: Terminal.Gui/ViewBase/View.Keyboard.cs:322-337

Current Implementation:

public bool NewKeyDownEvent (Key k)
{
    if (OnKeyDown (k) || k.Handled)
    {
        return true;
    }

    KeyDown?.Invoke (this, k);

    return k.Handled;
}

Can Use Helper?: ✅ YES - Clean fit

Proposed Migration:

public bool NewKeyDownEvent (Key k)
{
    ResultEventArgs<Key> args = new (k);
    
    bool? result = CWPWorkflowHelper.Execute(
        onMethod: args => OnKeyDown(k),
        eventHandler: (sender, e) => KeyDown?.Invoke(sender, k),
        args: args,
        defaultAction: null);

    return result == true || k.Handled;
}

Migration Difficulty: ⭐⭐ MEDIUM

Issues:

1. Key is mutable (k.Handled), not immutable like ResultEventArgs expects
2. Need to synchronize k.Handled with result
3. Event signature doesn't match helper's expected EventHandler<ResultEventArgs>
4. Current implementation is actually simpler

Recommendation: ⚠️ Marginal benefit. Could migrate for consistency but adds complexity.

---

2.2 OnKeyDownNotHandled / KeyDownNotHandled

File: Terminal.Gui/ViewBase/View.Keyboard.cs:339-349

Analysis: Same pattern as OnKeyDown

Can Use Helper?: ✅ YES (same issues)

Migration Difficulty: ⭐⭐ MEDIUM

Recommendation: ⚠️ Marginal benefit

---

2.3 OnKeyUp / KeyUp

File: Terminal.Gui/ViewBase/View.Keyboard.cs (similar location)

Analysis: Same pattern as OnKeyDown

Can Use Helper?: ✅ YES (same issues)

Migration Difficulty: ⭐⭐ MEDIUM

Recommendation: ⚠️ Marginal benefit

---

Category 3: Command Events (MEDIUM PRIORITY)

3.1 OnAccepting / Accepting

File: Terminal.Gui/ViewBase/View.Command.cs:160-177

Current Implementation:

protected bool? RaiseAccepting (ICommandContext? ctx)
{
    CommandEventArgs args = new () { Context = ctx };

    if (OnAccepting (args) || args.Handled)
    {
        return true;
    }

    Accepting?.Invoke (this, args);

    return Accepting is null ? null : args.Handled;
}

Can Use Helper?: ✅ YES - Good fit!

Proposed Migration:

protected bool? RaiseAccepting (ICommandContext? ctx)
{
    ResultEventArgs<ICommandContext?> args = new (ctx);
    
    return CWPWorkflowHelper.Execute(
        onMethod: args => OnAccepting(new CommandEventArgs { Context = args.Result }),
        eventHandler: (sender, e) => 
        {
            var cmdArgs = new CommandEventArgs { Context = e.Result };
            Accepting?.Invoke(sender, cmdArgs);
            e.Handled = cmdArgs.Handled;
        },
        args: args,
        defaultAction: null);
}

Migration Difficulty: ⭐⭐⭐ HARD

Issues:

1. CommandEventArgs is separate from ResultEventArgs
2. Need to create/map CommandEventArgs inside helper calls
3. Complex argument marshalling
4. Current implementation is clearer

Recommendation: ⚠️ Could work but adds complexity. Direct implementation is simpler.

---

3.2 OnSelecting / Selecting

File: Terminal.Gui/ViewBase/View.Command.cs:220-236

Analysis: Identical pattern to OnAccepting

Can Use Helper?: ✅ YES (same issues)

Migration Difficulty: ⭐⭐⭐ HARD

Recommendation: ⚠️ Same as OnAccepting

---

3.3 OnHandlingHotKey / HandlingHotKey

File: Terminal.Gui/ViewBase/View.Command.cs:260-277

Analysis: Identical pattern to OnAccepting

Can Use Helper?: ✅ YES (same issues)

Migration Difficulty: ⭐⭐⭐ HARD

Recommendation: ⚠️ Same as OnAccepting

---

3.4 OnCommandNotBound / CommandNotBound

File: Terminal.Gui/ViewBase/View.Command.cs:93-109

Analysis: Identical pattern to OnAccepting

Can Use Helper?: ✅ YES (same issues)

Migration Difficulty: ⭐⭐⭐ HARD

Recommendation: ⚠️ Same as OnAccepting

---

Category 4: Navigation Events (MEDIUM PRIORITY)

4.1 OnAdvancingFocus / AdvancingFocus

File: Terminal.Gui/ViewBase/View.Navigation.cs (AdvanceFocus method)

Current Implementation Pattern:

// Inside AdvanceFocus method
AdvanceFocusEventArgs args = new (direction, behavior);

if (OnAdvancingFocus (direction, behavior))
{
    return false;
}

AdvancingFocus?.Invoke (this, args);

if (args.Cancel)
{
    return false;
}

Can Use Helper?: ⚠️ PARTIAL - Embedded in larger method

Migration Difficulty: ⭐⭐⭐⭐ VERY HARD

Issues:

1. Not in its own Raise method - embedded in AdvanceFocus logic
2. Complex navigation state management around the pattern
3. Would require extracting to separate method first
4. Return semantics differ (returns bool, not bool?)

Recommendation: ❌ Don't migrate. Too embedded in complex logic.

---

4.2 OnHasFocusChanging / HasFocusChanging

File: Terminal.Gui/ViewBase/View.Navigation.cs:717-750

Analysis: Similar to AdvancingFocus - embedded in SetHasFocus method

Can Use Helper?: ⚠️ PARTIAL - Embedded in larger method

Migration Difficulty: ⭐⭐⭐⭐ VERY HARD

Recommendation: ❌ Don't migrate. Too embedded.

---

Category 5: Drawing Events (LOW PRIORITY)

5.1-5.6 Drawing Events

Files: Terminal.Gui/ViewBase/View.Drawing.cs

• OnDrawingAdornments / DrawingAdornments (line 284)
• OnClearingViewport / ClearingViewport (line 319)
• OnDrawingText / DrawingText (lines 407, 413)
• OnDrawingContent / DrawingContent (lines 489, 495)
• OnDrawingSubViews / DrawingSubViews (lines 545, 551)
• OnRenderingLineCanvas / RenderingLineCanvas (line 611)

Current Implementation Pattern (OnDrawingContent example):

private void DoDrawContent (DrawContext? context = null)
{
    if (OnDrawingContent (context))
    {
        return;
    }

    if (OnDrawingContent ())
    {
        return;
    }

    var dev = new DrawEventArgs (Viewport, Rectangle.Empty, context);
    DrawingContent?.Invoke (this, dev);

    if (dev.Cancel)
    {
        return;
    }

    // Actual drawing logic...
}

Can Use Helper?: ⚠️ PARTIAL - Non-standard pattern

Migration Difficulty: ⭐⭐⭐ HARD

Issues:

1. TWO virtual methods (overloaded) - OnDrawingContent(context) and OnDrawingContent()
2. DrawEventArgs created after virtual methods called
3. Embedded in DoDrawXxx methods with actual drawing logic after
4. Cancel semantics differ from standard CWP

Recommendation: ❌ Don't migrate. Custom drawing pipeline better as-is.

---

Category 6: Property Changes (MEDIUM PRIORITY)

6.1 OnVisibleChanging / VisibleChanging

File: Terminal.Gui/ViewBase/View.cs:382-420

Current Implementation:

public bool Visible
{
    get => _visible;
    set
    {
        if (_visible == value)
        {
            return;
        }

        if (OnVisibleChanging ())
        {
            return;
        }

        VisibleChanging?.Invoke (this, new CancelEventArgs (ref _visible, value));

        if (_visible == value)
        {
            return;
        }

        bool wasVisible = _visible;
        _visible = value;

        OnVisibleChanged ();
        VisibleChanged?.Invoke (this, EventArgs.Empty);
        
        // Post-processing...
        if (!_visible)
        {
            RestoreFocus ();
        }
        
        SetNeedsDraw ();
        // ... more post-processing
    }
}

Can Use Helper?: ✅ YES - CWPPropertyHelper designed for this!

Proposed Migration:

public bool Visible
{
    get => _visible;
    set
    {
        bool changed = CWPPropertyHelper.ChangeProperty(
            _visible,
            value,
            args => OnVisibleChanging(),  // Problem: signature mismatch
            VisibleChanging,               // Problem: uses CancelEventArgs, not ValueChangingEventArgs
            args => OnVisibleChanged(),
            VisibleChanged,                // Problem: uses EventArgs, not ValueChangedEventArgs
            out bool finalValue);

        if (changed)
        {
            _visible = finalValue;
            
            // Post-processing
            if (!_visible)
            {
                RestoreFocus ();
            }
            SetNeedsDraw ();
            // ... more
        }
    }
}

Migration Difficulty: ⭐⭐⭐⭐ VERY HARD

Issues:

1. OnVisibleChanging() takes no parameters - CWPPropertyHelper expects ValueChangingEventArgs
2. VisibleChanging uses CancelEventArgs, not ValueChangingEventArgs
3. VisibleChanged uses EventArgs, not ValueChangedEventArgs
4. Would require changing event signatures (BREAKING CHANGE)
5. Complex post-processing logic after change

Recommendation: ❌ Don't migrate. Would require breaking API changes.

---

6.2 OnSchemeNameChanging / SchemeName

File: Terminal.Gui/ViewBase/View.Drawing.Scheme.cs:25-44

Current Implementation: ✅ ALREADY USES CWPPropertyHelper!

Status: ✅ Already migrated - good example to follow

---

6.3 OnGettingScheme / GettingScheme

File: Terminal.Gui/ViewBase/View.Drawing.Scheme.cs:133-162

Current Implementation: ✅ ALREADY USES CWPWorkflowHelper.ExecuteWithResult!

Status: ✅ Already migrated - good example to follow

---

6.4 OnSettingScheme / SetScheme

File: Terminal.Gui/ViewBase/View.Drawing.Scheme.cs:217-234

Current Implementation: ✅ ALREADY USES CWPPropertyHelper!

Status: ✅ Already migrated - good example to follow

---

Category 7: View-Specific Events (LOW PRIORITY)

7.1 OnCellActivated / CellActivated (TableView)

File: Terminal.Gui/Views/TableView/TableView.cs:1277+

Analysis: View-specific, likely custom implementation

Can Use Helper?: ✅ Likely YES

Migration Difficulty: ⭐⭐ MEDIUM

Recommendation: ⚠️ Low priority, could migrate

---

7.2 OnPositionChanging (ScrollBar, ScrollSlider)

Files:

• Terminal.Gui/Views/ScrollBar/ScrollBar.cs:375
• Terminal.Gui/Views/ScrollBar/ScrollSlider.cs:245

Can Use Helper?: ✅ Likely YES - property change pattern

Migration Difficulty: ⭐⭐ MEDIUM

Issues:

1. Takes (int currentPos, int newPos) parameters
2. CWPPropertyHelper expects single value
3. Would need wrapper

Recommendation: ⚠️ Could work but parameters don't match pattern well

---

7.3 OnCheckedStateChanging (CheckBox)

File: Terminal.Gui/Views/CheckBox.cs:187

Current Implementation (in ChangeCheckedState):

private bool? ChangeCheckedState (CheckState value)
{
    if (_checkedState == value || (value is CheckState.None && !AllowCheckStateNone))
    {
        return null;
    }

    ResultEventArgs<CheckState> e = new (value);

    if (OnCheckedStateChanging (e))
    {
        return true;
    }

    CheckedStateChanging?.Invoke (this, e);

    if (e.Handled)
    {
        return e.Handled;
    }

    _checkedState = value;
    UpdateTextFormatterText ();
    SetNeedsLayout ();

    EventArgs<CheckState> args = new (in _checkedState);
    OnCheckedStateChanged (args);

    CheckedStateChanged?.Invoke (this, args);

    return false;
}

Can Use Helper?: ✅ YES - This is a PERFECT match!

Proposed Migration:

private bool? ChangeCheckedState (CheckState value)
{
    // Pre-condition check
    if (_checkedState == value || (value is CheckState.None && !AllowCheckStateNone))
    {
        return null;
    }

    bool changed = CWPPropertyHelper.ChangeProperty(
        _checkedState,
        value,
        OnCheckedStateChanging,
        CheckedStateChanging,
        OnCheckedStateChanged,
        CheckedStateChanged,
        out CheckState finalValue);

    if (changed)
    {
        _checkedState = finalValue;
        UpdateTextFormatterText();
        SetNeedsLayout();
        return false;
    }
    
    return true; // Was cancelled
}

Migration Difficulty: ⭐ EASY

Issues:

1. Need to adjust OnCheckedStateChanged signature from EventArgs to ValueChangedEventArgs
2. Return semantics: method returns bool? (null/true/false), helper only returns bool (true/false)

Recommendation: ✅ STRONGLY RECOMMEND - This is the best candidate! But need to handle:

• Signature changes for OnCheckedStateChanged
• Return value mapping (helper returns bool, method needs bool?)

---

7.4 OnGettingAttributeForRole / GettingAttributeForRole

File: Terminal.Gui/ViewBase/View.Drawing.Attribute.cs:78

Analysis: Likely embedded in GetAttributeForRole method

Can Use Helper?: ⚠️ PARTIAL - Need to examine implementation

Migration Difficulty: ⭐⭐⭐ HARD (unknown)

Recommendation: ⚠️ Low priority, need more analysis

---

7.5 OnBorderStyleChanged

File: Terminal.Gui/ViewBase/View.Adornments.cs:162

Analysis: Property change notification (not cancellable)

Can Use Helper?: ❌ NO - Post-change only, no pre-change cancellation

Recommendation: ❌ Don't migrate. Not CWP pattern.

---

7.6 OnMouseIsHeldDownTick (MouseHeldDown)

File: Terminal.Gui/ViewBase/MouseHeldDown.cs:71

Analysis: Specialized repeating tick event

Can Use Helper?: ❌ NO - Custom timer logic

Recommendation: ❌ Don't migrate. Specialized use case.

---

Migration Recommendations by Priority

✅ STRONGLY RECOMMEND (1 instance)

Effort: 1-2 days

1. CheckBox.OnCheckedStateChanging
   • Perfect fit for CWPPropertyHelper
   • Requires minor signature adjustments
   • Good example for others
   • Clear improvement

⚠️ CONSIDER (4 instances)

Effort: 1-2 weeks

2. Command Events (OnAccepting, OnSelecting, OnHandlingHotKey, OnCommandNotBound)

   • Could work but requires wrapper logic
   • Marginal improvement over direct implementation
   • Good for consistency if pursuing helper migration strategy

3. Keyboard Events (OnKeyDown, OnKeyDownNotHandled, OnKeyUp)

   • Could work but mutable Key complicates
   • Marginal improvement
   • Consider only for consistency

❌ DON'T MIGRATE (23 instances)

Reason: Custom logic, embedded patterns, or breaking changes required

• Mouse Events (4): OnMouseEvent, OnMouseClick, OnMouseWheel, OnMouseEnter

  • Custom pre/post processing
  • Direct implementation clearer

• Navigation Events (2): OnAdvancingFocus, OnHasFocusChanging

  • Embedded in complex methods
  • Too tightly coupled

• Drawing Events (6): All drawing events

  • Custom pipeline
  • Non-standard pattern with dual overloads

• Property Changes (1): OnVisibleChanging

  • Would require breaking API changes
  • Complex post-processing

• View-Specific (10): TableView, ScrollBar/Slider (2), BorderStyle, MouseHeldDown, etc.

  • Specialized logic
  • Low value

Overall Migration Strategy

Option A: Minimal Migration (RECOMMENDED)

Effort: 1-2 days Risk: LOW

Migrate only:

1. CheckBox.OnCheckedStateChanging (perfect fit)

Benefits:

• Demonstrates helper usage in real View code
• Clear improvement
• Low risk

Drawbacks:

• Limited consistency improvement
• Most code still uses direct implementation

---

Option B: Partial Migration

Effort: 2-3 weeks Risk: MEDIUM

Migrate:

1. CheckBox.OnCheckedStateChanging
2. All Command events (4 instances)
3. All Keyboard events (3 instances)

Benefits:

• More consistency
• Demonstrates helper usage in core events
• Shows pattern across event types

Drawbacks:

• Significant refactoring effort
• Marginal improvement for many instances
• Requires careful testing
• More complex code in some cases

---

Option C: Full Migration

Effort: 6-8 weeks Risk: HIGH

Attempt to migrate all possible instances.

Benefits:

• Maximum consistency
• Full helper usage

Drawbacks:

• Many instances don't fit helper pattern well
• Would require API breaking changes
• Creates wrapper complexity
• Direct implementation often clearer
• Very high effort for limited benefit

Recommendation: ❌ NOT RECOMMENDED

---

Key Findings & Gotchas

1. Helper Pattern Assumptions Don't Always Match Reality

Issue: Helpers assume clean separation of concerns, but real implementations have:

• Pre-conditions (Enabled checks, visibility checks)
• Post-conditions (command invocation, state updates)
• Complex argument types (CommandEventArgs vs ResultEventArgs)
• Mutable arguments (Key.Handled)

Example: RaiseMouseClickEvent has both pre-checks and post-command invocation that can't fit in helper.

---

2. Event Argument Type Mismatches

Issue: Many events use custom EventArgs types that don't match helper expectations:

• CancelEventArgs vs ValueChangingEventArgs
• CommandEventArgs vs ResultEventArgs
• EventArgs vs ValueChangedEventArgs

Impact: Would require:

• Breaking API changes to event signatures, OR
• Complex wrapper logic that negates helper benefits

---

3. Embedded Patterns Resist Extraction

Issue: Many CWP patterns are embedded in larger methods (AdvanceFocus, DoDrawContent, etc.) with:

• Complex surrounding logic
• State management
• Multiple phases

Impact: Extracting to use helpers would:

• Require significant refactoring
• Reduce readability
• Increase complexity

---

4. Two-Virtual-Method Pattern

Issue: Drawing events have TWO virtual methods:

OnDrawingContent(DrawContext? context)
OnDrawingContent()  // No parameters

Impact: Helper assumes single virtual method. This pattern can't fit without major changes.

---

5. Return Value Semantics Differ

Issue: Many methods return bool?, helpers return bool or bool?:

• null = no subscribers
• true = handled/cancelled
• false = not handled

Impact: Translating return values adds complexity.

---

6. Current Helper Usage is Limited

Fact: Only View.Drawing.Scheme.cs uses helpers currently (3 usages).

Implications:

• Helpers are relatively new
• Most code predates helpers
• Helpers designed for specific patterns (property changes, result workflows)
• Not designed to replace all CWP implementations

---

7. Direct Implementation Often Clearer

Observation: For many instances, direct implementation is:

• More readable
• More maintainable
• Less code
• Fewer abstractions

Example:

// Direct (Current) - Clear and simple
if (OnKeyDown (k) || k.Handled)
{
    return true;
}
KeyDown?.Invoke (this, k);
return k.Handled;

// With Helper - More complex
ResultEventArgs<Key> args = new (k);
bool? result = CWPWorkflowHelper.Execute(
    onMethod: args => OnKeyDown(k),
    eventHandler: (sender, e) => KeyDown?.Invoke(sender, k),
    args: args,
    defaultAction: null);
return result == true || k.Handled;

---

Recommendations Summary

For Maintainers

1. Don't pursue full migration - Effort (6-8 weeks) far exceeds benefit
2. Migrate CheckBox.OnCheckedStateChanging - Perfect fit, good example
3. Consider Command/Keyboard events - Only if pursuing consistency strategy
4. Keep direct implementations - For most instances, direct is clearer
5. Document when to use helpers - Update guidance for new CWP implementations

For New Code

Use CWPPropertyHelper when:

• Implementing property setter with validation
• Need pre/post change events
• Event types match ValueChangingEventArgs and ValueChangedEventArgs

Use CWPWorkflowHelper when:

• Simple workflow with single result type
• Clean separation of virtual method, event, default action
• No complex pre/post processing
• ResultEventArgs fits naturally

Use direct implementation when:

• Custom pre/post processing logic
• Complex argument types
• Embedded in larger method
• Event types don't match helper expectations
• Direct code is clearer

Documentation Updates Needed

If pursuing any migration:

1. Update docs/analysis/cwp_analysis_report.md

   • Add section on helper class usage
   • Document migration outcomes

2. Update docfx/docs/events.md

   • Add guidance on when to use helpers
   • Show examples of helper usage
   • Document when direct implementation is better

3. Update docfx/docs/cancellable-work-pattern.md

   • Add helper class section
   • Explain design decisions
   • Show pros/cons

Conclusion

Total Effort Summary:

• Option A (Recommended): 1-2 days
• Option B (Partial): 2-3 weeks
• Option C (Full): 6-8 weeks (not recommended)

Key Insight: CWP helper classes were designed for specific patterns (property changes, clean workflows) and work well for those cases (View.Drawing.Scheme.cs). However, most CWP implementations in the codebase have custom logic, pre/post processing, or embedded patterns that don't fit the helper abstraction well.

Recommendation: Migrate CheckBox.OnCheckedStateChanging only (Option A). This demonstrates helper usage in a real view, provides a clear example for developers, and improves code quality without significant risk or effort. Leave other implementations as direct implementations since they're often clearer and more maintainable that way.

The goal should be to use helpers where they add value, not to force all code into the helper pattern.