Skip to content

Commit 35093c5

Browse files
Refactor ToolStripRenderer classes and improve generic and GDIPlus Copilot Instruction files.
1 parent e8cb721 commit 35093c5

File tree

6 files changed

+724
-404
lines changed

6 files changed

+724
-404
lines changed

.github/Copilot/GDIPlus-copilot-instructions.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,23 @@ internal abstract partial class RefCountedCache<TObject, TCacheEntryData, TKey>
1414

1515
These cached objects have implicit conversions that make them behave like actual GDI+ types while significantly reducing allocation overhead.
1616

17+
### APIs, which already provide cached Pens or Brushes.
18+
19+
The APIs `SystemPens` and `SystemBrushes` already provide cached Pen/Brush objects.
20+
- Make sure you prioritize their usage (except you need pen objects with a different width).
21+
- If you use this API, these Pen or Brush object MUST NOT be disposed!
22+
23+
For example:
24+
```CSharp
25+
// Nothing to explicitly cache, since `SystemPens` already provides cached pen objects.
26+
if (ToolStripManager.VisualStylesEnabled)
27+
{
28+
e.Graphics.DrawLine(SystemPens.ButtonHighlight, 0, bounds.Bottom - 1, bounds.Width, bounds.Bottom - 1);
29+
e.Graphics.DrawLine(SystemPens.InactiveBorder, 0, bounds.Bottom - 2, bounds.Width, bounds.Bottom - 2);
30+
}
31+
```
32+
33+
1734
### 1.2 Using Cached Objects
1835

1936
Always prefer cached objects over direct instantiation:

.github/copilot-instructions.md

Lines changed: 229 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,43 @@
2626
- **Always insert empty lines after structure blocks and before `return` statements**
2727

2828
- Use pattern matching, `is`, `and`, `or`, and switch expressions where applicable
29+
2930
```csharp
3031
if (obj is Control control && control.Visible)
3132
{
3233
// Use 'control' variable
3334
}
34-
35+
```
36+
37+
- Refactor or use the ternary operator where possible, but make sure that you put each branch on its own line, except if the expression is very short:
38+
39+
```csharp
40+
// Original code
41+
textColor = e.Item.Enabled ? GetDarkModeColor(e.TextColor) : GetDarkModeColor(SystemColors.GrayText);
42+
43+
// or:
44+
if (e.Item.Enabled)
45+
{
46+
textColor = GetDarkModeColor(e.TextColor);
47+
}
48+
else
49+
{
50+
textColor = GetDarkModeColor(SystemColors.GrayText);
51+
}
52+
```
53+
54+
becomes:
55+
56+
```csharp
57+
// Use ternary operator with line breaks for clarity
58+
textColor = e.Item.Enabled
59+
? GetDarkModeColor(e.TextColor)
60+
: GetDarkModeColor(SystemColors.GrayText);
61+
```
62+
63+
- - Prefer `switch` expression over switch statements over chains of `if` statements, where if makes sense:
64+
65+
```csharp
3566
// Prefer switch expressions over switch statements
3667
string result = value switch
3768
{
@@ -41,7 +72,131 @@
4172
};
4273
```
4374

75+
Chain of If-statements can often be converted like so:
76+
77+
```csharp
78+
// Original code
79+
private static Color GetDarkModeColor(Color color)
80+
{
81+
if (color == SystemColors.Control)
82+
return Color.FromArgb(45, 45, 45);
83+
if (color == SystemColors.ControlLight)
84+
return Color.FromArgb(60, 60, 60);
85+
if (color == SystemColors.ControlDark)
86+
return Color.FromArgb(30, 30, 30);
87+
if (color == SystemColors.ControlText)
88+
return Color.FromArgb(240, 240, 240);
89+
// For any other colors, darken them if they're bright
90+
if (color.GetBrightness() > 0.5)
91+
{
92+
// Create a darker version for light colors
93+
return ControlPaint.Dark(color, 0.2f);
94+
}
95+
96+
return color;
97+
}
98+
```
99+
100+
becomes:
101+
102+
```csharp
103+
// Use switch expression for color handling
104+
private static Color GetDarkModeColor(Color color) =>
105+
color switch
106+
{
107+
Color c when c == SystemColors.Control => Color.FromArgb(45, 45, 45),
108+
Color c when c == SystemColors.ControlLight => Color.FromArgb(60, 60, 60),
109+
Color c when c == SystemColors.ControlDark => Color.FromArgb(30, 30, 30),
110+
_ when color.GetBrightness() > 0.5 => ControlPaint.Dark(color, 0.2f),
111+
_ => color
112+
};
113+
```
114+
115+
Other examples for switch expressions are:
116+
// Multiple conditions with 'and'/'or' patterns
117+
string GetFileCategory(string extension) => extension.ToLower() switch
118+
{
119+
".jpg" or ".png" or ".gif" or ".bmp" => "Image",
120+
".mp4" or ".avi" or ".mkv" or ".mov" => "Video",
121+
".txt" or ".md" or ".csv" => "Text",
122+
string ext when ext.StartsWith(".doc") => "Document",
123+
_ => "Unknown"
124+
};
125+
126+
// Tuple patterns for multiple parameters
127+
string GetQuadrant(int x, int y) => (x, y) switch
128+
{
129+
(> 0, > 0) => "First",
130+
(< 0, > 0) => "Second",
131+
(< 0, < 0) => "Third",
132+
(> 0, < 0) => "Fourth",
133+
_ => "Origin or Axis"
134+
};
135+
136+
// Property patterns with nested conditions
137+
decimal CalculateShipping(Order order) => order switch
138+
{
139+
{ Weight: > 50, IsPriority: true } => 25.00m,
140+
{ Weight: > 50 } => 15.00m,
141+
{ IsPriority: true, Total: > 100 } => 5.00m,
142+
{ Total: > 100 } => 0m,
143+
_ => 7.50m
144+
};
145+
146+
// Type patterns with casting
147+
string ProcessValue(object value) => value switch
148+
{
149+
int i when i > 0 => $"Positive: {i}",
150+
int i when i < 0 => $"Negative: {i}",
151+
string s when !string.IsNullOrEmpty(s) => $"Text: {s}",
152+
bool b => b ? "True" : "False",
153+
null => "Null value",
154+
_ => "Unknown type"
155+
};
156+
157+
// Range patterns (C# 11+)
158+
string GetAgeGroup(int age) => age switch
159+
{
160+
< 13 => "Child",
161+
>= 13 and < 20 => "Teen",
162+
>= 20 and < 65 => "Adult",
163+
>= 65 => "Senior",
164+
_ => "Invalid"
165+
};
166+
167+
// List patterns (C# 11+) - great for method parameter validation
168+
string AnalyzeList<T>(List<T> items) => items switch
169+
{
170+
[] => "Empty",
171+
[var single] => $"Single item: {single}",
172+
[var first, var second] => $"Two items: {first}, {second}",
173+
[var first, .., var last] => $"Multiple items from {first} to {last}",
174+
};
175+
176+
// Enum with flags - often overlooked refactoring opportunity
177+
string GetPermissionDescription(FilePermissions permissions) => permissions switch
178+
{
179+
FilePermissions.Read => "Read only",
180+
FilePermissions.Write => "Write only",
181+
FilePermissions.Read | FilePermissions.Write => "Read and Write",
182+
FilePermissions.Read | FilePermissions.Execute => "Read and Execute",
183+
FilePermissions.All => "Full access",
184+
_ => "No permissions"
185+
};
186+
187+
// Refactoring complex validation chains
188+
ValidationResult ValidateUser(User user) => user switch
189+
{
190+
null => ValidationResult.Error("User cannot be null"),
191+
{ Email: null or "" } => ValidationResult.Error("Email required"),
192+
{ Email: string email } when !email.Contains('@') => ValidationResult.Error("Invalid email"),
193+
{ Age: < 13 } => ValidationResult.Error("Must be 13 or older"),
194+
{ Name.Length: < 2 } => ValidationResult.Error("Name too short"),
195+
_ => ValidationResult.Success()
196+
};
197+
44198
- **Prefer modern collection initializers**: Use `[]` syntax for collections
199+
45200
```csharp
46201
List<string> items = []; // Preferred
47202
List<string> items = new(); // Also acceptable
@@ -235,6 +390,7 @@
235390
- **Remove trailing whitespace** (enforced as warning)
236391
- **Insert final newline at end of files**
237392
- **Use 4-space indentation consistently**
393+
238394
- **Place opening braces on new lines** (Allman style)
239395
```csharp
240396
if (condition)
@@ -243,6 +399,64 @@
243399
}
244400
```
245401

402+
### 1.10 Magic Numbers
403+
404+
- **Avoid magic numbers**: Use named constants or enums instead of hard-coded values. This improves readability and maintainability.
405+
```csharp
406+
// Avoid magic numbers
407+
const int MaxItems = 100;
408+
const double Pi = 3.14159;
409+
410+
// Use named constants or enums
411+
public enum ColorCode
412+
{
413+
Red = 1,
414+
Green = 2,
415+
Blue = 3
416+
}
417+
418+
// Example usage
419+
ColorCode color = ColorCode.Red;
420+
```
421+
422+
### 1.11 Using of empty lines for clarity
423+
424+
- **Use empty lines to separate logical sections of code**: This improves readability and helps to understand the structure of the code. Use them judiciously to avoid cluttering the code.
425+
426+
```csharp
427+
// Example of using empty lines for clarity
428+
public int SomeMethod()
429+
{
430+
if (condition)
431+
{
432+
// Do something
433+
}
434+
else
435+
{
436+
// Do something else.
437+
// Leave a line after the closing brace for clarity.
438+
}
439+
440+
// Initialize variables
441+
int x = 0;
442+
int y = 0;
443+
444+
// Perform calculations
445+
x = CalculateX();
446+
447+
// Update UI, and the return in a separate line.
448+
UpdateUI(x, y);
449+
450+
return x;
451+
}
452+
```
453+
454+
### 1.12 Refactoring of existing comments
455+
456+
- **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.
457+
458+
- Do not delete comments, though, when they carry either necessary information or are helpful for the reader. Instead, refactor them to be more precise and clear.
459+
246460
## 2. WinForms-Specific Guidelines
247461

248462
### 2.1 Event Handling
@@ -257,6 +471,20 @@
257471
}
258472
```
259473

474+
- **Use `EventArgs.Empty` for empty event arguments**
475+
```csharp
476+
protected virtual void OnPaint(PaintEventArgs e)
477+
{
478+
Paint?.Invoke(this, EventArgs.Empty);
479+
}
480+
```
481+
482+
- Take into account, that the typical Event handler signature has been modified for NRT:
483+
```csharp
484+
private void OnClick(object? sender, EventArgs e) { ... }
485+
```
486+
487+
260488
### 2.2 Designer Integration
261489

262490
- **Mark designer-generated fields appropriately**

src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripRenderer.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,12 +1130,14 @@ private protected static void OnRenderStatusStripSizingGrip(
11301130
if (Environment.OSVersion.Version >= new Version(10, 0, 22000)
11311131
&& statusStrip.FindForm() is Form f)
11321132
{
1133+
#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.
11331134
cornerDef = f.FormCornerPreference switch
11341135
{
11351136
FormCornerPreference.Round => (4, new(1, 1, 2, 2)),
11361137
FormCornerPreference.RoundSmall => (3, new(1, 1, 2, 2)),
11371138
_ => (2, new(0, 0, 2, 2))
11381139
};
1140+
#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.
11391141
}
11401142

11411143
return cornerDef;

0 commit comments

Comments
 (0)