Update clang-format to 10.0 #7389
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit removes our local copy of clang-format 8 and replaces it with a newly-built nuget package containing clang-format 10. This resolves the inconsistency between our version of clang-format and the one shipped in Visual Studio. A couple minor format changes were either required or erroneously forced upon us--chief among them is a redistribution of `*`s around SAL annotations in inline class members of COM classes. Don't ask why; I couldn't figure it out. We had some aspirational goals for our formatting, which were left in but commented out. Enabling them changes our format a little more than I'm comfortable with, so I uncommented them and locked them to the format style we've been using for the past year. We may not love it, but our aspirations may not matter here any longer. Consistent formatting is better than perfect formatting.
DHowett
commented
Aug 25, 2020
| @@ -70,5 +70,5 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" | |||
| // 1. static_map's member types are all the same | |||
| // 2. static_map's fourth template argument (otherwise undeduced) is how many pairs it contains | |||
| template<typename First, typename... Rest> | |||
| static_map(First, Rest...)->static_map<std::conditional_t<std::conjunction_v<std::is_same<First, Rest>...>, typename First::first_type, void>, typename First::second_type, std::less<typename First::first_type>, 1 + sizeof...(Rest)>; | |||
| static_map(First, Rest...) -> static_map<std::conditional_t<std::conjunction_v<std::is_same<First, Rest>...>, typename First::first_type, void>, typename First::second_type, std::less<typename First::first_type>, 1 + sizeof...(Rest)>; | |||
There was a problem hiding this comment.
FORMAT: deduction guide gets a space around its arrow.
DHowett
commented
Aug 25, 2020
| @@ -242,7 +242,7 @@ HRESULT HwndTerminal::Initialize() | |||
| _terminal->Create(COORD{ 80, 25 }, 1000, *_renderer); | |||
| _terminal->SetDefaultBackground(RGB(12, 12, 12)); | |||
| _terminal->SetDefaultForeground(RGB(204, 204, 204)); | |||
| _terminal->SetWriteInputCallback([=](std::wstring & input) noexcept { _WriteTextToConnection(input); }); | |||
| _terminal->SetWriteInputCallback([=](std::wstring& input) noexcept { _WriteTextToConnection(input); }); | |||
There was a problem hiding this comment.
FORMAT: clang-format used to (weirdly) force the space before the &, now it forces no space
DHowett
commented
Aug 25, 2020
| @@ -34,7 +34,7 @@ class ConsolePropertySheetHandler WrlFinal : public RuntimeClass<RuntimeClassFla | |||
| } | |||
|
|
|||
| // IPersist | |||
| STDMETHODIMP GetClassID(_Out_ CLSID* clsid) override | |||
| STDMETHODIMP GetClassID(_Out_ CLSID * clsid) override | |||
There was a problem hiding this comment.
FORMAT: This whole file is filled with the issues I called out in the body.
There was a problem hiding this comment.
There is some serious Objective-C vibe going around this one...
There was a problem hiding this comment.
Ah, objective-C, my other favorite language :)
DHowett
force-pushed
the
dev/duhowett/clang-format-10
branch
from
c78ad05
to
807c26d
Compare
zadjii-msft
approved these changes
Aug 25, 2020
miniksa
reviewed
Aug 25, 2020
Comment on lines
+358
to
+359
| $clangPackage = ([xml](Get-Content "$env:OpenConsoleRoot\tools\packages.config")).packages.package | Where-Object id -like "clang-format*" | ||
| $clangFormatPath = "$env:OpenConsoleRoot\packages\$($clangPackage.id).$($clangPackage.version)\tools\clang-format.exe" |
miniksa
approved these changes
Aug 25, 2020
miniksa
added
the
AutoMerge
|
Hello @miniksa! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
DHowett
added a commit
that referenced
this pull request
Sep 3, 2020
Dustin L. Howett * Clear the last error before calling Mb2Wc in ConvertToW (GH-7391) * Update clang-format to 10.0 (GH-7389) * Add til::static_map, a constexpr key-value store (GH-7323) James Holderness * Refactor VT control sequence identification (CC-7304) Mike Griese * Compensate for VS 16.7, part 2 (GH-7383) * Add support for iterable, nested commands (GH-6856) Michael Niksa * Helix Testing (GH-6992) * Compensate for new warnings and STL changes in VS 16.7 (GH-7319) nathpete-msft * Fix environment block creation (GH-7401) Chester Liu * Add initial support for VT DCS sequences (CC-6328) Related work items: #28791050
DHowett
added a commit
that referenced
this pull request
Sep 18, 2020
This commit removes our local copy of clang-format 8 and replaces it with a newly-built nuget package containing clang-format 10. This resolves the inconsistency between our version of clang-format and the one shipped in Visual Studio. A couple minor format changes were either required or erroneously forced upon us--chief among them is a redistribution of `*`s around SAL annotations in inline class members of COM classes. Don't ask why; I couldn't figure it out. We had some aspirational goals for our formatting, which were left in but commented out. Enabling them changes our format a little more than I'm comfortable with, so I uncommented them and locked them to the format style we've been using for the past year. We may not love it, but our aspirations may not matter here any longer. Consistent formatting is better than perfect formatting. (cherry picked from commit dbbe820)
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit removes our local copy of clang-format 8 and replaces it
with a newly-built nuget package containing clang-format 10.
This resolves the inconsistency between our version of clang-format and
the one shipped in Visual Studio.
A couple minor format changes were either required or erroneously forced
upon us--chief among them is a redistribution of
*s around SALannotations in inline class members of COM classes. Don't ask why; I
couldn't figure it out.
We had some aspirational goals for our formatting, which were left in
but commented out. Enabling them changes our format a little more than
I'm comfortable with, so I uncommented them and locked them to the
format style we've been using for the past year. We may not love it, but
our aspirations may not matter here any longer. Consistent formatting is
better than perfect formatting.