Update coding/file style guidelines #3923
Conversation
paulmedynski
added
the
Area\Engineering
There was a problem hiding this comment.
Pull request overview
Updates repository-wide formatting defaults and documents new style guidelines (encoding, line endings, and line length) while keeping C# indentation at 4 spaces.
Changes:
- Update
.editorconfigdefaults (2-space indent, LF, UTF-8, trim trailing whitespace) with explicit overrides for C# and Windows scripts. - Refresh
policy/coding-style.mdto add new repo-wide text file rules and wrap lines to ~100 chars. - Remove redundant file-type-specific
.editorconfigsections now covered by the new defaults.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| policy/coding-style.md | Adds/rewraps guidelines (UTF-8/LF, 100-char lines) and reformats the rules list. |
| .editorconfig | Changes default formatting settings and simplifies per-file-type overrides (keeps C# at 4 spaces, Windows scripts CRLF). |
| use braces (See rule 17 for more details). One exception is that a `using` statement is permitted | ||
| to be nested within another `using` statement by starting on the following line at the same | ||
| indentation level, even if the nested `using` contains a controlled block. |
There was a problem hiding this comment.
The Allman-brace rule still says "See rule 17 for more details", but with the new auto-numbered list and the two new rules inserted above, the single-statement if guidance is no longer rule 17. This cross-reference is now incorrect/misleading; consider referencing the target rule by name (e.g., "See the single-statement if rule below") or updating the rule number to match the new ordering.
| use braces (See rule 17 for more details). One exception is that a `using` statement is permitted | |
| to be nested within another `using` statement by starting on the following line at the same | |
| indentation level, even if the nested `using` contains a controlled block. | |
| use braces (see the single-statement `if` rule below for more details). One exception is that a | |
| `using` statement is permitted to be nested within another `using` statement by starting on the | |
| following line at the same indentation level, even if the nested `using` contains a controlled block. |
| The general rule we follow is "_use Visual Studio defaults_". | ||
|
|
||
| 1. We use [Allman style](http://en.wikipedia.org/wiki/Indent_style#Allman_style) braces, where each brace begins on a new line. A single line statement block can go without braces but the block must be properly indented on its own line and must not be nested in other statement blocks that use braces (See rule 17 for more details). One exception is that a `using` statement is permitted to be nested within another `using` statement by starting on the following line at the same indentation level, even if the nested `using` contains a controlled block. | ||
| 2. We use four spaces of indentation (no tabs). | ||
| 3. We use `_camelCase` for internal and private fields and use `readonly` where possible. Prefix internal and private instance fields with `_`, static fields with `s_` and thread static fields with `t_`. When used on static fields, `readonly` should come after `static` (e.g. `static readonly` not `readonly static`). Public fields should be used sparingly and should use PascalCasing with no prefix when used. | ||
| 4. We avoid `this.` unless absolutely necessary. | ||
| 5. We always specify the visibility, even if it's the default (e.g. `private string _foo` not `string _foo`). Visibility should be the first modifier (e.g. `public abstract` not `abstract public`). | ||
| 6. Namespace imports should be specified at the top of the file, *outside* of `namespace` declarations, and should be sorted alphabetically, with the exception of `System.*` namespaces, which are to be placed on top of all others. | ||
| 7. Avoid more than one empty line at any time. For example, do not have two blank lines between members of a type. | ||
| 8. Avoid spurious free spaces. For example avoid `if (someVar == 0)...`, where the dots mark the spurious free spaces. Consider enabling "View White Space (Ctrl+R, Ctrl+W)" or "Edit -> Advanced -> View White Space" if using Visual Studio to aid detection. | ||
| 9. If a file happens to differ in style from these guidelines (e.g. private members are named `m_member` rather than `_member`), the existing style in that file takes precedence. | ||
| 10. We only use `var` when it's obvious what the variable type is (e.g. `var stream = new FileStream(...)` not `var stream = OpenStandardInput()`). | ||
| 11. We use language keywords instead of BCL types (e.g. `int, string, float` instead of `Int32, String, Single`, etc) for both type references as well as method calls (e.g. `int.Parse` instead of `Int32.Parse`). | ||
| 12. We use PascalCasing to name all our constant local variables and fields. The only exception is for interop code where the constant value should exactly match the name and value of the code you are calling via interop. | ||
| 13. We use ```nameof(...)``` instead of ```"..."``` whenever possible and relevant. | ||
| 14. Fields should be specified at the top within type declarations. | ||
| 15. When including non-ASCII characters in the source code use Unicode escape sequences (\uXXXX) instead of literal characters. Literal non-ASCII characters occasionally get garbled by a tool or editor. | ||
| 16. When using labels (for goto), indent the label one less than the current indentation. | ||
| 17. When using a single-statement if, we follow these conventions: | ||
| - Never use single-line form (for example: `if (source == null) throw new ArgumentNullException("source");`) | ||
| - Using braces is always accepted, and required if any block of an `if`/`else if`/.../`else` compound statement uses braces or if a single statement body spans multiple lines. | ||
| - Braces may be omitted only if the body of *every* block associated with an `if`/`else if`/.../`else` compound statement is placed on a single line. | ||
| 1. Text files should be UTF-8 encoded, no BOM, and use Unix line endings (LF). The exception is | ||
| Windows script files, which may use CRLF. | ||
| 1. Lines should be a maximum of 100 characters. Exceptions are made for long content such as URLs, | ||
| and when breaking text would be less readable. |
There was a problem hiding this comment.
The statement "The general rule we follow is "use Visual Studio defaults"" now conflicts with the new repo-wide text-file requirements (LF endings, UTF-8, etc.) and the updated .editorconfig defaults. Consider rewording this to clarify that .editorconfig is the source of truth for repo formatting defaults (and that VS is expected to honor it).
| - Never use single-line form (for example: `if (source == null) throw new ArgumentNullException("source");`) | ||
| - Using braces is always accepted, and required if any block of an `if`/`else if`/.../`else` compound statement uses braces or if a single statement body spans multiple lines. | ||
| - Braces may be omitted only if the body of *every* block associated with an `if`/`else if`/.../`else` compound statement is placed on a single line. | ||
| 1. Text files should be UTF-8 encoded, no BOM, and use Unix line endings (LF). The exception is |
There was a problem hiding this comment.
I added these 2 new rules. The remaining changes are line-wrapping to the new 100 standard.
| charset = utf-8 | ||
|
|
||
| # Trim trailing whitespace. | ||
| trim_trailing_whitespace = true |
There was a problem hiding this comment.
This may be a bit controversial, since it will add a bunch of whitespace diffs to our PRs as we touch files with trailing whitespace. Easy to ignore with the per-PR setting in GitHub, so I'd prefer to keep it and slowly get our files cleaned up.
| [*.{yml,yaml}] | ||
| indent_size = 2 | ||
| # Use Unix-style line endings. | ||
| end_of_line = lf |
There was a problem hiding this comment.
I think most of our files are already LF, so this seemed to make sense. VSCode won't change an existing file automatically.
| end_of_line = lf | ||
|
|
||
| # UTF-8 encoding without BOM. | ||
| charset = utf-8 |
There was a problem hiding this comment.
We have some files with BOM, and this may remove the BOM when we touch those files. No biggie IMO.
| - Braces may be omitted only if the body of *every* block associated with an `if`/`else if`/.../`else` compound statement is placed on a single line. | ||
| 1. Text files should be UTF-8 encoded, no BOM, and use Unix line endings (LF). The exception is | ||
| Windows script files, which may use CRLF. | ||
| 1. Lines should be a maximum of 100 characters. Exceptions are made for long content such as URLs, |
There was a problem hiding this comment.
I went with 100 here to see how it goes. I can fit 2 panes of 100-max code, along with an agent chat, and the explorer on my screen. If lines are longer, they will wrap and get messy. Otherwise, I will need a 50-inch 8K monitor to see everything comfortably 😆
Changes to default settings:
C# specific:
Windows scripts:
Removed sections:
Coding style:
New guidelines added:
Formatting improvements: