starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-09-24 07:54:35 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Coding Standards: Switch to using in-class initializers instead of initializers in constructors #1284

New Issue
Open
opened 2026-01-30 22:21:08 +00:00 by claunia · 6 comments
claunia commented 2026-01-30 22:21:08 +00:00
Owner
Copy Link

Originally created by @zadjii-msft on GitHub (May 23, 2019).

From discussion in #825. We should probably make a point to move existing code over to this style. @adiviness might have ideas on the best way to go about this.

The relevant discussion is copied below:

dlong11 5 days ago

@zadjii-msft Some of these variables should be class initialized.
C.48: Prefer in-class initializers to member initializers in constructors for constant initializers

zadjii-msft 3 days ago Author Member

Considering this is the style that we're consistently using throughout the rest of the codebase, I'm going to leave these.

Seems kinda odd that we've been converting things to use this style when this isn't the standard, but I'm not the standards guru here - @adiviness is

adiviness 2 days ago Member

The reasoning for doing all initialization in the constructor is to prevent splitting the initialization code across two files, it's a little easier to read when it's all done in the constructor. Ideally we could get around the chance of missing initialization of a field with a compiler warning like what clang has. This may be something we can revisit one the static analysis tools are in place.

dlong11 2 days ago

@zadjii-msft @adiviness @DHowett-MSFT @binarycrusader I wasn't sure who this comment should go to so I've added everyone I have interacted with.

From an outsiders point of view - It is very difficult to review code when you don't know what the coding standards are. I have seen that you follow the Cpp Core Guidelines but only in certain areas? Maybe this comes from the fact that you have a distinction from "old" code vs "new" code. I believe it may come from the clause - If it's inserting something into the existing classes/functions, try to follow the existing style as closely as possible. I understand the reasoning of this but this can be a real trap when you are dealing with coding standards.

This essentially means you have two coding standards and now you must know when to apply each. This may be easy for someone who is on the "team" but this can be a deal breaker for the outside community. When and how will this code get updated to the new standards? Why would we continually add code debt? Cleaning this code up in the past may have been impossible due to the team size, but now you have a community that can/will do this if they know the rules.

This is a good example -

The reasoning for doing all initialization in the constructor is to prevent splitting the initialization code across two files, it's a little easier to read when it's all done in the constructor.

This is contradictory to C.48: Prefer in-class initializers to member initializers in constructors for constant initializers

I am fine with either approach, but to effectively review this code we need to know which set of rules we are following.

I don't want this to come off as ranting. This is purely feedback and not meant to cast stones at anyone! I am just trying to figure out the rules and expectations. (I'm sure others are as well) Also, I am not trying to push for one standard vs another. Since this project has been newly opened up, it is entirely possible that you guys are still figuring this out. I can understand that.

I thought it might be helpful to give you an outsiders point of view.

zadjii-msft a day ago Author Member

This is a very valid point, and I want to make sure you don't feel like we are just dismissing this opinion. Updating our code to modern standards has been a long and arduous journey, but we're definitely not all there yet (evidently). I think where we're mostly coming from is a very Windows-centric mindset, where we have a huge codebase with varying styles across all of it. In Windows, the guidance* has been to match the existing style when writing code in an existing codebase, and try to conform to the standard when writing a new feature areas.

I definitely hear you that this is a major pain points for external contributors, and something that we really need to work with the community on going forward. There's #890, which is probably a good thread to copy most of this commentary into.

adiviness a day ago Member

Typically how we've done stylistic things in the past is that new code gets written in whatever we considered the best of coding standards/styling at the time, but we also don't know everything about C++ :) I'm actively looking into getting code formatting/linting/static analysis/etc. tools set up for the project. For something like the initializers I'd expect that we'd enable whatever check that makes a tool yell at us about them and then we'd fix all of the instances in one fell swoop. Although I'm not sure if it's possible for a tool to find instances of this particular coding guideline quite yet.

We do want to more closely follow the coding standards we've referenced, this may be something worth opening an issue for so switching the initializers could be tracked.

dlong11 17 hours ago

@zadjii-msft Thanks for the reply.

and I want to make sure you don't feel like we are just dismissing this opinion.

I don't feel like anyone is dismissing my opinion. You guys have been great. I understand that we are in forming/storming stage. All good.

Updating our code to modern standards has been a long and arduous journey, but we're definitely not all there yet (evidently). I think where we're mostly coming from is a very Windows-centric mindset, where we have a huge codebase with varying styles across all of it.

Too funny. I am in the same boat. I have been a working a large legacy windows-centric code base for a long time. A few years ago we decided to go cross platform and start to update to Modern C++. So we were in the same boat. It has been a long road and obviously when you have millions of lines of code you can't clean it all up at once. We started with a similar set of rules like you. "keep the code consistent, ....." What we found was doing this is almost impossible. It is really hard to say "follow these guidelines for this code" and "follow these for this code".

In a nutshell this is what helped us get there.

Have a style guide and follow it for all new and modified code. The more detail the better.
I mean really follow it. :) This sounds funny but until everyone commits to it and understands it you will have issues. All devs/reviewers need to understand the Core guidelines (or whatever your coding standards are) - this takes some time to get there but it is doable.

Compile with all Major compilers - Clang has great sanitizers, better warnings, etc...

Use clang-format! This will save a huge amount of time. No more - oh you have an extra line here, you have the braces in the wrong spot, etc... Also, be OK if it is not exactly the same as what you do now. Believe me, it is worth it.

Setup external tools like the CppCoreGuilines analyzer / static analyzer - this sounds great but with a large code base, it is difficult to run the analyzer and fix them all. In this repo you may be able to do that. The terminal codebase isn't that large.

Document - Contributing.md, style guide, Do's / Dont's, etc. If everyone knows what the rules are then everyone will be more productive. Also realize the community doesn't have the years of backstory that msft devs have. (this is easy to forget) Doing a good job of this will pay huge dividends.

There's #890, which is probably a good thread to copy most of this commentary into.

I opened this. 👍 For this repo to be successful this stuff is a must.

You guys seem to have a good start on this and @adiviness has a lot of this on his list. So you guys are on the right track. I'm looking forward to watching this repo mature.

Originally created by @zadjii-msft on GitHub (May 23, 2019). From discussion in #825. We should probably make a point to move existing code over to this style. @adiviness might have ideas on the best way to go about this. The relevant discussion is copied below: <hr> dlong11 5 days ago @zadjii-msft Some of these variables should be class initialized. C.48: Prefer in-class initializers to member initializers in constructors for constant initializers zadjii-msft 3 days ago Author Member Considering this is the style that we're consistently using throughout the rest of the codebase, I'm going to leave these. Seems kinda odd that we've been converting things to use this style when this isn't the standard, but I'm not the standards guru here - @adiviness is adiviness 2 days ago Member The reasoning for doing all initialization in the constructor is to prevent splitting the initialization code across two files, it's a little easier to read when it's all done in the constructor. Ideally we could get around the chance of missing initialization of a field with a compiler warning like what clang has. This may be something we can revisit one the static analysis tools are in place. dlong11 2 days ago @zadjii-msft @adiviness @DHowett-MSFT @binarycrusader I wasn't sure who this comment should go to so I've added everyone I have interacted with. From an outsiders point of view - It is very difficult to review code when you don't know what the coding standards are. I have seen that you follow the Cpp Core Guidelines but only in certain areas? Maybe this comes from the fact that you have a distinction from "old" code vs "new" code. I believe it may come from the clause - If it's inserting something into the existing classes/functions, try to follow the existing style as closely as possible. I understand the reasoning of this but this can be a real trap when you are dealing with coding standards. This essentially means you have two coding standards and now you must know when to apply each. This may be easy for someone who is on the "team" but this can be a deal breaker for the outside community. When and how will this code get updated to the new standards? Why would we continually add code debt? Cleaning this code up in the past may have been impossible due to the team size, but now you have a community that can/will do this if they know the rules. This is a good example - The reasoning for doing all initialization in the constructor is to prevent splitting the initialization code across two files, it's a little easier to read when it's all done in the constructor. This is contradictory to C.48: Prefer in-class initializers to member initializers in constructors for constant initializers I am fine with either approach, but to effectively review this code we need to know which set of rules we are following. I don't want this to come off as ranting. This is purely feedback and not meant to cast stones at anyone! I am just trying to figure out the rules and expectations. (I'm sure others are as well) Also, I am not trying to push for one standard vs another. Since this project has been newly opened up, it is entirely possible that you guys are still figuring this out. I can understand that. I thought it might be helpful to give you an outsiders point of view. zadjii-msft a day ago Author Member This is a very valid point, and I want to make sure you don't feel like we are just dismissing this opinion. Updating our code to modern standards has been a long and arduous journey, but we're definitely not all there yet (evidently). I think where we're mostly coming from is a very Windows-centric mindset, where we have a huge codebase with varying styles across all of it. In Windows, the guidance* has been to match the existing style when writing code in an existing codebase, and try to conform to the standard when writing a new feature areas. I definitely hear you that this is a major pain points for external contributors, and something that we really need to work with the community on going forward. There's #890, which is probably a good thread to copy most of this commentary into. adiviness a day ago Member Typically how we've done stylistic things in the past is that new code gets written in whatever we considered the best of coding standards/styling at the time, but we also don't know everything about C++ :) I'm actively looking into getting code formatting/linting/static analysis/etc. tools set up for the project. For something like the initializers I'd expect that we'd enable whatever check that makes a tool yell at us about them and then we'd fix all of the instances in one fell swoop. Although I'm not sure if it's possible for a tool to find instances of this particular coding guideline quite yet. We do want to more closely follow the coding standards we've referenced, this may be something worth opening an issue for so switching the initializers could be tracked. dlong11 17 hours ago @zadjii-msft Thanks for the reply. and I want to make sure you don't feel like we are just dismissing this opinion. I don't feel like anyone is dismissing my opinion. You guys have been great. I understand that we are in forming/storming stage. All good. Updating our code to modern standards has been a long and arduous journey, but we're definitely not all there yet (evidently). I think where we're mostly coming from is a very Windows-centric mindset, where we have a huge codebase with varying styles across all of it. Too funny. I am in the same boat. I have been a working a large legacy windows-centric code base for a long time. A few years ago we decided to go cross platform and start to update to Modern C++. So we were in the same boat. It has been a long road and obviously when you have millions of lines of code you can't clean it all up at once. We started with a similar set of rules like you. "keep the code consistent, ....." What we found was doing this is almost impossible. It is really hard to say "follow these guidelines for this code" and "follow these for this code". In a nutshell this is what helped us get there. Have a style guide and follow it for all new and modified code. The more detail the better. I mean really follow it. :) This sounds funny but until everyone commits to it and understands it you will have issues. All devs/reviewers need to understand the Core guidelines (or whatever your coding standards are) - this takes some time to get there but it is doable. Compile with all Major compilers - Clang has great sanitizers, better warnings, etc... Use clang-format! This will save a huge amount of time. No more - oh you have an extra line here, you have the braces in the wrong spot, etc... Also, be OK if it is not exactly the same as what you do now. Believe me, it is worth it. Setup external tools like the CppCoreGuilines analyzer / static analyzer - this sounds great but with a large code base, it is difficult to run the analyzer and fix them all. In this repo you may be able to do that. The terminal codebase isn't that large. Document - Contributing.md, style guide, Do's / Dont's, etc. If everyone knows what the rules are then everyone will be more productive. Also realize the community doesn't have the years of backstory that msft devs have. (this is easy to forget) Doing a good job of this will pay huge dividends. There's #890, which is probably a good thread to copy most of this commentary into. I opened this. 👍 For this repo to be successful this stuff is a must. You guys seem to have a good start on this and @adiviness has a lot of this on his list. So you guys are on the right track. I'm looking forward to watching this repo mature. <hr>
claunia commented 2026-01-30 22:21:09 +00:00
Author
Owner
Copy Link

@adiviness commented on GitHub (May 23, 2019):

As I get more styling tooling online we might find that something will yell at us about it but I'm not 100% sure a tool exists for that yet.

@adiviness commented on GitHub (May 23, 2019): As I get more styling tooling online we might find that something will yell at us about it but I'm not 100% sure a tool exists for that yet.
claunia commented 2026-01-30 22:21:10 +00:00
Author
Owner
Copy Link

@dlong11 commented on GitHub (May 24, 2019):

@adiviness I am not aware of any tooling that will catch this. (doesn't mean it doesn't exist) The Core Guidelines analyzer doesn't flag this. We could add C26495 - this won't catch this case but it would catch all of the unitialized class variables. If anything gets flagged we could start to apply this rule.

@dlong11 commented on GitHub (May 24, 2019): @adiviness I am not aware of any tooling that will catch this. (doesn't mean it doesn't exist) The Core Guidelines analyzer doesn't flag this. We could add [C26495](https://docs.microsoft.com/en-us/visualstudio/code-quality/c26495?f1url=https%3A%2F%2Fmsdn.microsoft.com%2Fquery%2Fdev15.query%3FappId%3DDev15IDEF1%26l%3DEN-US%26k%3Dk(C26495)%26rd%3Dtrue&view=vs-2019) - this won't catch this case but it would catch all of the unitialized class variables. If anything gets flagged we could start to apply this rule.
claunia commented 2026-01-30 22:21:10 +00:00
Author
Owner
Copy Link

@binarycrusader commented on GitHub (May 24, 2019):

As I get more styling tooling online we might find that something will yell at us about it but I'm not 100% sure a tool exists for that yet.

OACR will suggest use of delegated constructors (what you're already using) but last I checked was woefully unprepared to deal with member initializers.

@binarycrusader commented on GitHub (May 24, 2019): > As I get more styling tooling online we might find that something will yell at us about it but I'm not 100% sure a tool exists for that yet. OACR will suggest use of delegated constructors (what you're already using) but last I checked was woefully unprepared to deal with member initializers.
claunia commented 2026-01-30 22:21:10 +00:00
Author
Owner
Copy Link

@ratnesh-12 commented on GitHub (Oct 2, 2025):

Hey, I'd like to work on this as my first contribution :)

@ratnesh-12 commented on GitHub (Oct 2, 2025): Hey, I'd like to work on this as my first contribution :)
claunia commented 2026-01-30 22:21:10 +00:00
Author
Owner
Copy Link

@04suriya07-spec commented on GitHub (Dec 14, 2025):

I’d like to take a pass at this if no one else is already working on it.

My plan would be:

Propose a clear, written rule for member initialization (aligned with C++ Core Guidelines C.48) and add it to
style/contributing docs.
Prefer in-class member initializers for constant/default values in all new and modified code.
Identify a clang-tidy configuration that gets us as close as possible to enforcing this automatically.
(Optionally) follow up with a focused cleanup PR once the rule and tooling are agreed on.

I’m intentionally not planning to mass-update existing code until there’s consensus on the rule and tooling.

If someone is already working on this, or if this approach doesn’t align with current plans, please let me know before I start.

@04suriya07-spec commented on GitHub (Dec 14, 2025): > I’d like to take a pass at this if no one else is already working on it. My plan would be: > > Propose a clear, written rule for member initialization (aligned with C++ Core Guidelines C.48) and add it to style/contributing docs. > > Prefer in-class member initializers for constant/default values in all new and modified code. > > Identify a clang-tidy configuration that gets us as close as possible to enforcing this automatically. > >(Optionally) follow up with a focused cleanup PR once the rule and tooling are agreed on. I’m intentionally not planning to mass-update existing code until there’s consensus on the rule and tooling. If someone is already working on this, or if this approach doesn’t align with current plans, please let me know before I start.
claunia commented 2026-01-30 22:21:11 +00:00
Author
Owner
Copy Link

@shivam-dhir commented on GitHub (Jan 22, 2026):

Hi, I would love to work on this issue

@shivam-dhir commented on GitHub (Jan 22, 2026): Hi, I would love to work on this issue
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-CodeHealth good first issue Help Wanted In-PR Issue-Task Product-Meta
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#1284
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?