|
| 1 | +# Contributing to GhostDraw |
| 2 | + |
| 3 | +Thanks for your interest in contributing to **GhostDraw**! 🎉 |
| 4 | + |
| 5 | +This document explains how to propose changes, what we expect from contributions, and some project-specific guidelines (especially around safety, since GhostDraw installs global hooks and overlays your desktop). |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## 1. Ways to Contribute |
| 10 | + |
| 11 | +- 🐛 **Report bugs** via [GitHub Issues](https://github.com/RuntimeRascal/ghost-draw/issues) |
| 12 | +- 💡 **Request features** (new tools, usability improvements, settings, etc.) |
| 13 | +- 💻 **Code contributions** (bug fixes, features, refactors) |
| 14 | +- 🧪 **Tests** (unit tests, edge-case coverage) |
| 15 | +- 📝 **Docs** (README, `docs/`, in-app help) |
| 16 | + |
| 17 | +Before starting larger work, please: |
| 18 | +- Check existing **issues** and **PRs** to avoid duplicates |
| 19 | +- Comment on an issue you want to tackle, or open a new one describing your proposal |
| 20 | + |
| 21 | +--- |
| 22 | + |
| 23 | +## 2. Safety First (Critical for This Project) |
| 24 | + |
| 25 | +GhostDraw uses **global keyboard and mouse hooks** and displays a fullscreen transparent overlay. This means bugs can affect basic system interaction. |
| 26 | + |
| 27 | +Please follow these *non‑negotiable* rules: |
| 28 | + |
| 29 | +1. **Never block the hook chain** |
| 30 | + - Hook callbacks must always call `CallNextHookEx`. |
| 31 | + - Return quickly (< 5 ms) and avoid heavy work in hooks. |
| 32 | + |
| 33 | +2. **Handle all exceptions defensively** |
| 34 | + - Wrap critical paths (hooks, overlay activation, settings load/save) in `try/catch`. |
| 35 | + - Log exceptions; never let them crash the process without cleanup. |
| 36 | + |
| 37 | +3. **Overlay must never trap the user** |
| 38 | + - `ESC` must always provide a way to escape drawing mode. |
| 39 | + - On unhandled exceptions, hide the overlay and release hooks. |
| 40 | + |
| 41 | +4. **Always clean up on exit** |
| 42 | + - Unhook global hooks. |
| 43 | + - Dispose timers, event handlers, and unmanaged resources. |
| 44 | + |
| 45 | +5. **Log appropriately** |
| 46 | + - Use `ILogger<T>` and Serilog. |
| 47 | + - Info/Debug for normal flow; Error/Critical for failures. |
| 48 | + - Avoid logging on every mouse move—this can flood logs and impact performance. |
| 49 | + |
| 50 | +If in doubt, prioritize **stability and escape paths** over features. |
| 51 | + |
| 52 | +--- |
| 53 | + |
| 54 | +## 3. Development Setup |
| 55 | + |
| 56 | +### Prerequisites |
| 57 | + |
| 58 | +- **OS**: Windows 10 or later |
| 59 | +- **.NET SDK**: .NET 8 SDK |
| 60 | +- **IDE**: Visual Studio, Rider, or VS Code with C# extension |
| 61 | + |
| 62 | +### Clone and Build |
| 63 | + |
| 64 | +```powershell |
| 65 | +# Clone |
| 66 | +git clone https://github.com/RuntimeRascal/ghost-draw.git |
| 67 | +cd ghost-draw |
| 68 | +
|
| 69 | +# Build app + tests |
| 70 | +dotnet build |
| 71 | +
|
| 72 | +# Run tests |
| 73 | +dotnet test |
| 74 | +``` |
| 75 | + |
| 76 | +The main application project is in `Src/GhostDraw/`. Tests live in `Tests/GhostDraw.Tests/`. |
| 77 | + |
| 78 | +--- |
| 79 | + |
| 80 | +## 4. Branch & PR Workflow |
| 81 | + |
| 82 | +1. **Create an issue** (or pick an existing one) |
| 83 | + - Describe the bug/feature and scope clearly. |
| 84 | +2. **Create a feature branch** from `main`: |
| 85 | + |
| 86 | + ```powershell |
| 87 | + git checkout main |
| 88 | + git pull |
| 89 | + git checkout -b my-feature-branch |
| 90 | + ``` |
| 91 | + |
| 92 | +3. **Implement your changes** |
| 93 | + - Keep changes focused on the issue. |
| 94 | + - Avoid unrelated refactors in the same PR. |
| 95 | + |
| 96 | +4. **Add / Update tests** |
| 97 | + - New features should have test coverage. |
| 98 | + - Bug fixes should include regression tests when feasible. |
| 99 | + |
| 100 | +5. **Run tests locally** before opening a PR: |
| 101 | + |
| 102 | + ```powershell |
| 103 | + dotnet test |
| 104 | + ``` |
| 105 | + |
| 106 | +6. **Open a Pull Request** |
| 107 | + - Reference the issue (e.g., `Fixes #123`). |
| 108 | + - Summarize the change and any risks. |
| 109 | + - Mention anything that needs extra validation (installer, multi‑monitor, high DPI, etc.). |
| 110 | + |
| 111 | +### 4.1 Alternative (Copilot-Powered) Workflow ☕ |
| 112 | + |
| 113 | +If you’d rather spend more time sipping coffee than wiring up boilerplate: |
| 114 | + |
| 115 | +1. **Create a detailed issue** describing the bug/feature |
| 116 | + - Include context, expected behavior, edge cases, and screenshots if helpful. |
| 117 | +2. **Assign a GitHub Copilot agent** to the issue |
| 118 | + - Let the agent spin up a branch, implement the changes, and open a PR for you. |
| 119 | +3. **Go get a cup of coffee** (or two ☕☕) |
| 120 | +4. **Come back and review the PR** |
| 121 | + - Run tests locally |
| 122 | + - Double-check safety guidelines (hooks, overlay behavior, ESC escape paths) |
| 123 | + - Request tweaks or follow-ups as needed |
| 124 | + |
| 125 | +--- |
| 126 | + |
| 127 | +## 5. Coding Guidelines |
| 128 | + |
| 129 | +### General |
| 130 | + |
| 131 | +- Target **.NET 8**. |
| 132 | +- Prefer **explicit namespaces** over ambiguous type usages when WPF/WinForms overlap (`Point`, `Color`, etc.). |
| 133 | +- Use **dependency injection** for services (`ILogger<T>`, settings services, etc.). |
| 134 | +- Avoid static state unless it is truly global and safe. |
| 135 | + |
| 136 | +### WPF / UI |
| 137 | + |
| 138 | +- Keep UI logic in `Views/` and behavior/logic in code‑behind or view models as appropriate. |
| 139 | +- For icons in XAML: |
| 140 | + - Use **Segoe MDL2 Assets** (`FontFamily="Segoe MDL2 Assets"`). |
| 141 | + - Use hex codes like `` for chevrons, checkmarks, etc. |
| 142 | + - Avoid raw Unicode emojis inside WPF controls; they may render as `?` on some systems. |
| 143 | + |
| 144 | +### Hooks & Input |
| 145 | + |
| 146 | +- Never perform heavy work directly in hook callbacks. |
| 147 | +- Use events or dispatch to the UI thread for anything non‑trivial. |
| 148 | +- Always ensure `CallNextHookEx` (or equivalent) is called. |
| 149 | + |
| 150 | +### Logging |
| 151 | + |
| 152 | +- Use structured logging with message templates: |
| 153 | + |
| 154 | + ```csharp |
| 155 | + _logger.LogInformation("Drawing mode {State}", enabled); |
| 156 | + _logger.LogError(ex, "Failed to load settings"); |
| 157 | + ``` |
| 158 | + |
| 159 | +- Do not log every mouse move or wheel event at Info level—use Trace/Debug sparingly. |
| 160 | + |
| 161 | +--- |
| 162 | + |
| 163 | +## 6. Testing Guidelines |
| 164 | + |
| 165 | +When changing behavior, especially around input or overlay behavior, please: |
| 166 | + |
| 167 | +- Add/extend unit tests in `Tests/GhostDraw.Tests/`. |
| 168 | +- Favor fast, deterministic tests (no real hooks or UI interaction where possible). |
| 169 | +- For higher‑risk changes, perform manual checks: |
| 170 | + - Drawing on multiple monitors |
| 171 | + - High DPI scaling |
| 172 | + - Rapid hotkey toggling |
| 173 | + - Using ESC to recover from mistakes |
| 174 | + |
| 175 | +Example test command: |
| 176 | + |
| 177 | +```powershell |
| 178 | +dotnet test Tests/GhostDraw.Tests/GhostDraw.Tests.csproj |
| 179 | +``` |
| 180 | + |
| 181 | +--- |
| 182 | + |
| 183 | +## 7. Installer Changes |
| 184 | + |
| 185 | +If you modify the WiX installer (`Installer/`): |
| 186 | + |
| 187 | +- Ensure user settings in `%LOCALAPPDATA%/GhostDraw` are **not** deleted on uninstall. |
| 188 | +- Keep ICE warnings under control (or explicitly suppressed with rationale). |
| 189 | +- Test install, upgrade, and uninstall flows on a real Windows machine. |
| 190 | + |
| 191 | +--- |
| 192 | + |
| 193 | +## 8. Documentation |
| 194 | + |
| 195 | +For user‑visible changes, update: |
| 196 | + |
| 197 | +- `README.md` – high‑level features and screenshots |
| 198 | +- `CHANGELOG.md` – versioned history (follow existing format) |
| 199 | +- `docs/` – detailed behavior docs (e.g., key legend, feature plans) |
| 200 | +- F1 help overlay – if you add or change shortcuts/tools |
| 201 | + |
| 202 | +--- |
| 203 | + |
| 204 | +## 9. Code Review Expectations |
| 205 | + |
| 206 | +PRs are more likely to be merged quickly if they: |
| 207 | + |
| 208 | +- Are **small and focused** |
| 209 | +- Include tests and docs updates |
| 210 | +- Explain **why** the change is needed, not just what it does |
| 211 | +- Call out any known limitations or follow‑up work |
| 212 | + |
| 213 | +We may ask for changes to maintain safety, consistency, or clarity. |
| 214 | + |
| 215 | +--- |
| 216 | + |
| 217 | +## 10. Questions? |
| 218 | + |
| 219 | +If you’re unsure about anything: |
| 220 | + |
| 221 | +- Open a **discussion** or **issue** on GitHub with your questions. |
| 222 | +- Propose your approach before implementing large or risky changes. |
| 223 | + |
| 224 | +Thanks again for helping make GhostDraw better! 👻💜 |
0 commit comments