Skip to content

Commit 61aefaa

Browse files
Do final code-style pass to meet all standards and improve copilot instructions.
1 parent f713234 commit 61aefaa

File tree

12 files changed

+151
-173
lines changed

12 files changed

+151
-173
lines changed

.github/AI-Prompts/GDI-Refactoring/AIP-cache_pens-and_brushes.md

Lines changed: 0 additions & 80 deletions
This file was deleted.

.github/copilot-instructions.md

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,11 @@
215215

216216
- **Use `var` only for complex types when type is apparent from context**
217217
```csharp
218-
var control = new Button(); // OK - type is obvious
219-
var items = GetComplexCollection(); // OK - long type name
218+
var control = new Button(); // OK - type is obvious, but better is:
219+
Button control = new(); // Preferred
220+
221+
// OK - long type name
222+
var items = GetComplexCollectionWithRidiculousLongTypeName();
220223
int primitiveValue = 42; // Required for primitives
221224
```
222225

@@ -239,6 +242,11 @@
239242
```csharp
240243
private static readonly int s_defaultBorderWidth = 1;
241244
```
245+
An exception to this rule is when the field is attributed with `[ThreadStatic]`, in which case the prefix should be `t_`:
246+
```csharp
247+
[ThreadStatic]
248+
private static int t_threadLocalValue;
249+
```
242250

243251
- **Prefix private/internal instance fields with `_`** (suggestion level)
244252
```csharp
@@ -286,7 +294,7 @@
286294
private bool IsVisible() => Visible && Parent?.Visible == true;
287295
```
288296

289-
- For longer expressions, use line breaks with proper alignment:
297+
For longer expressions, use line breaks with proper alignment:
290298
```csharp
291299
internal int SomeFooIntegerProperty =>
292300
_someFooIntegerProperty;
@@ -295,6 +303,19 @@
295303
size.Width > 0
296304
&& size.Height > 0;
297305
```
306+
**IMPORTANT**
307+
308+
In C#, `public Foo FooInstance => new Foo();` and `public Foo FooInstance { get; } = new Foo();` are NOT interchangeable.
309+
310+
* The first is an expression-bodied property that returns a new value each time it is accessed.
311+
* The second is a read-only auto-property with initializer that returns the same value every time, created once.
312+
313+
When editing, generating, or suggesting C# properties, always maintain the original intent:
314+
315+
* Use an expression-bodied property (`=>`) for computed or dynamic values.
316+
* Use an auto-property with initializer (`{ get; } = ...`) for stored, initialized-once values.
317+
318+
Never "correct" or convert one style to the other unless you are certain the semantic behavior is intended to change because it was wrong with 100% confidence to begin with. If you encounter a case, where you have a expression-bodied property that returns a new instance, and you encounter this in a code review, you should add a comment explaining why this is intended, and that it is not a mistake.
298319

299320
### 1.5 Error Handling and Argument Validation
300321

@@ -345,19 +366,38 @@
345366
- **Handle platform-specific code appropriately**
346367
```csharp
347368
[SupportedOSPlatform("windows")]
348-
private void WindowsSpecificMethod() { ... }
369+
private void WindowsSpecificMethod()
370+
{
371+
...
372+
}
349373
```
350374

351375
- **Use proper disposal patterns**
352-
```csharp
353-
using var brush = new SolidBrush(Color.Red); // Simple using for single resources
354-
355-
// Traditional using for complex scenarios
356-
using (Graphics g = CreateGraphics())
357-
{
358-
// Drawing operations
359-
}
360-
```
376+
Prefer the modern C# “using declaration” syntax (using var ...) over the traditional using statement block.
377+
Use:
378+
379+
```csharp
380+
using var brush = backColor.GetCachedSolidBrushScope();
381+
```
382+
383+
instead of:
384+
385+
```csharp
386+
using (SolidBrushCache.Scope brush = backColor.GetCachedSolidBrushScope())
387+
{
388+
// ...
389+
}
390+
```
391+
392+
This makes code more concise, reduces nesting, and improves readability. Only use the older style if you need to limit the scope of the variable to a smaller section within a larger method, or when the using var pattern is not supported. **Note**, that if you use `using` with the type name over `var` for new instances that you can omit the type name after the `new` like:
393+
394+
```csharp
395+
using Pen focusPen = new(focusColor) // Use 'using' with type name and omit type after 'new'
396+
{
397+
Width = 1.0f,
398+
DashStyle = DashStyle.Dot
399+
};
400+
```
361401

362402
### 1.8 XML Documentation Standards
363403

@@ -402,6 +442,7 @@
402442
### 1.10 Magic Numbers
403443

404444
- **Avoid magic numbers**: Use named constants or enums instead of hard-coded values. This improves readability and maintainability.
445+
405446
```csharp
406447
// Avoid magic numbers
407448
const int MaxItems = 100;
@@ -451,6 +492,9 @@
451492
}
452493
```
453494

495+
Note: If a code line for which these empty-line rules apply has a comment on top of it, the empty line should be placed before the comment, if there isn't one. And of course, NEVER comment that an empty lines need to be inserted at a spot, which would defeat the purpose of the empty line.
496+
497+
454498
### 1.12 Refactoring of existing comments
455499

456500
- **Refactor existing comments**: When refactoring code, also consider updating or removing outdated comments. Comments should accurately reflect the current state of the code and its purpose. Avoid/rephrase inappropriate comments or microaggressions.

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonBase.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ protected ButtonBase()
6969
| ControlStyles.ResizeRedraw
7070
| ControlStyles.OptimizedDoubleBuffer
7171
// We gain about 2% in painting by avoiding extra GetWindowText calls
72-
| ControlStyles.CacheText,
72+
| ControlStyles.CacheText
73+
| ControlStyles.StandardClick,
7374
true);
7475

7576
// This class overrides GetPreferredSizeCore, let Control automatically cache the result

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonInternal/DarkMode/ButtonDarkModeAdapter.cs

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@ internal override void PaintUp(PaintEventArgs e, CheckState state)
2929
{
3030
try
3131
{
32-
var smoothingMode = e.Graphics.SmoothingMode;
33-
e.Graphics.SmoothingMode = Drawing.Drawing2D.SmoothingMode.AntiAlias;
32+
// Use GraphicsInternal for better performance (GDI+ best practice)
33+
var g = e.GraphicsInternal;
34+
var smoothingMode = g.SmoothingMode;
35+
g.SmoothingMode = Drawing.Drawing2D.SmoothingMode.AntiAlias;
3436

3537
LayoutData layout = CommonLayout().Layout();
3638

3739
ButtonDarkModeRenderer.RenderButton(
38-
e.Graphics,
40+
g,
3941
Control.ClientRectangle,
4042
Control.FlatStyle,
4143
ToPushButtonState(state, Control.Enabled),
@@ -52,7 +54,7 @@ internal override void PaintUp(PaintEventArgs e, CheckState state)
5254
drawFocus: false)
5355
);
5456

55-
e.Graphics.SmoothingMode = smoothingMode;
57+
g.SmoothingMode = smoothingMode;
5658
}
5759
catch (Exception)
5860
{
@@ -65,13 +67,14 @@ internal override void PaintDown(PaintEventArgs e, CheckState state)
6567
{
6668
try
6769
{
68-
// Set the smoothing mode to AntiAlias for better rendering quality
69-
var smoothingMode = e.Graphics.SmoothingMode;
70-
e.Graphics.SmoothingMode = Drawing.Drawing2D.SmoothingMode.AntiAlias;
70+
// Use GraphicsInternal for better performance (GDI+ best practice)
71+
var g = e.GraphicsInternal;
72+
var smoothingMode = g.SmoothingMode;
73+
g.SmoothingMode = Drawing.Drawing2D.SmoothingMode.AntiAlias;
7174

7275
LayoutData layout = CommonLayout().Layout();
7376
ButtonDarkModeRenderer.RenderButton(
74-
e.Graphics,
77+
g,
7578
Control.ClientRectangle,
7679
Control.FlatStyle,
7780
PushButtonState.Pressed,
@@ -88,8 +91,7 @@ internal override void PaintDown(PaintEventArgs e, CheckState state)
8891
drawFocus: false)
8992
);
9093

91-
// Restore the original smoothing mode
92-
e.Graphics.SmoothingMode = smoothingMode;
94+
g.SmoothingMode = smoothingMode;
9395
}
9496
catch (Exception)
9597
{
@@ -102,13 +104,14 @@ internal override void PaintOver(PaintEventArgs e, CheckState state)
102104
{
103105
try
104106
{
105-
// Set the smoothing mode to AntiAlias for better rendering quality
106-
var smoothingMode = e.Graphics.SmoothingMode;
107-
e.Graphics.SmoothingMode = Drawing.Drawing2D.SmoothingMode.AntiAlias;
107+
// Use GraphicsInternal for better performance (GDI+ best practice)
108+
var g = e.GraphicsInternal;
109+
var smoothingMode = g.SmoothingMode;
110+
g.SmoothingMode = Drawing.Drawing2D.SmoothingMode.AntiAlias;
108111

109112
LayoutData layout = CommonLayout().Layout();
110113
ButtonDarkModeRenderer.RenderButton(
111-
e.Graphics,
114+
g,
112115
Control.ClientRectangle,
113116
Control.FlatStyle,
114117
PushButtonState.Hot,
@@ -125,16 +128,18 @@ internal override void PaintOver(PaintEventArgs e, CheckState state)
125128
drawFocus: false)
126129
);
127130

128-
// Restore the original smoothing mode
129-
e.Graphics.SmoothingMode = smoothingMode;
131+
g.SmoothingMode = smoothingMode;
130132
}
131133
catch (Exception ex)
132134
{
133135
Debug.Assert(false, $"Exception in PaintOver: {ex.Message}");
134136
}
135137
}
136138

137-
protected override LayoutOptions Layout(PaintEventArgs e) => CommonLayout();
139+
protected override LayoutOptions Layout(PaintEventArgs e)
140+
{
141+
return CommonLayout();
142+
}
138143

139144
private new LayoutOptions CommonLayout()
140145
{

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonInternal/DarkMode/ButtonDarkModeRendererBase.cs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ internal abstract partial class ButtonDarkModeRendererBase : IButtonRenderer
2020
/// <param name="graphics">Graphics context to draw on</param>
2121
private static void ClearBackground(Graphics graphics, Color parentBackgroundColor)
2222
{
23+
ArgumentNullException.ThrowIfNull(graphics);
24+
2325
graphics.Clear(parentBackgroundColor);
2426
}
2527

@@ -57,12 +59,6 @@ public void RenderButton(
5759
// Draw button background and get content bounds
5860
Rectangle contentBounds = DrawButtonBackground(graphics, paddedBounds, state, isDefault);
5961

60-
// Offset content bounds for Popup style when button is pressed
61-
// if (flatStyle == FlatStyle.Popup && state == PushButtonState.Pressed)
62-
// {
63-
// contentBounds.Offset(1, 1);
64-
// }
65-
6662
// Paint image and field using the provided delegates
6763
paintImage(contentBounds);
6864

@@ -72,12 +68,6 @@ public void RenderButton(
7268
false);
7369

7470
DrawFocusIndicator(graphics, bounds, isDefault);
75-
76-
// if (focused && showFocusCues)
77-
// {
78-
// // Draw focus indicator for other styles
79-
// renderer.DrawFocusIndicator(graphics, contentBounds, isDefault);
80-
// }
8171
}
8272

8373
public abstract Rectangle DrawButtonBackground(Graphics graphics, Rectangle bounds, PushButtonState state, bool isDefault);

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonInternal/DarkMode/DarkModeAdapterFactory.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ namespace System.Windows.Forms.ButtonInternal;
66
internal static class DarkModeAdapterFactory
77
{
88
#pragma warning disable WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
9+
910
public static ButtonBaseAdapter CreateFlatAdapter(ButtonBase control)
1011
{
1112
return Application.IsDarkModeEnabled
@@ -26,5 +27,6 @@ public static ButtonBaseAdapter CreatePopupAdapter(ButtonBase control)
2627
? new ButtonDarkModeAdapter(control)
2728
: new ButtonPopupAdapter(control);
2829
}
30+
2931
#pragma warning restore WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
3032
}

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonInternal/DarkMode/FlatButtonDarkModeRenderer.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ private static void DrawButtonBorder(Graphics graphics, Rectangle bounds, PushBu
137137
}
138138

139139
// For other states, draw a border with appropriate thickness
140-
// For pressed state, draw a darker border
141140
borderColor = isDefault
142141
? IButtonRenderer.DarkModeButtonColors.DefaultSingleBorderColor
143142
: IButtonRenderer.DarkModeButtonColors.SingleBorderColor;

0 commit comments

Comments
 (0)