-
Notifications
You must be signed in to change notification settings - Fork 738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clang-format Mosh #1253
clang-format Mosh #1253
Conversation
src/network/compressor.h
Outdated
}; | ||
|
||
Compressor & get_compressor( void ); | ||
Compressor &get_compressor( void ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we were space-&-space before, I think we have an opportunity to do &-space rather than space-&. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am pleased to report that the clang-format file that @keithw linked resolves this.
src/network/network.h
Outdated
TO_SERVER = 0, | ||
TO_CLIENT = 1 | ||
}; | ||
enum Direction { TO_SERVER = 0, TO_CLIENT = 1 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these should be newline separated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
src/terminal/terminaldisplayinit.cc
Outdated
throw std::runtime_error( | ||
"Terminal is hardcopy and cannot be used by curses applications." ); | ||
break; | ||
case 0: throw std::runtime_error( "Unknown terminal type." ); break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the newlines here should also be persisted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
src/util/select.h
Outdated
{ | ||
/* These initializations are not used; they are just | ||
here to appease -Weffc++. */ | ||
, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comma should be on line 60, but that may be a clang-format self-stability issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also fixed by @keithw’s clang-format.
Cool! FWIW if you want a .clang-format that tries to approximate the "keithw coding style" that Mosh was written in, we've been using https://github.com/fix-project/fix/blob/master/.clang-format . But things have probably drifted; I'm not sure if the one in this PR is closer to Mosh-as-it-stands or the one I linked to. To be clear I think you should pick whatever style you want. |
I have reformatted using @keithw’s clang-format file. Thanks, Keith! |
Per discussion offline, I have set |
Create .clang-format to describe the current C++ style used in Mosh. Mark one carefully-formatted array with `// clang-format off`. Also turn off clang-format in src/crypto/ocb_internal.cc, since it was imported almost wholesale from another project and is written in a style different from the rest of Mosh.
Run clang-format over the Mosh source tree. This is a large change and has been factored into its own commit for auditability. Reproduce it with find . -name \*.cc -or -name \*.h | while read f; do clang-format -i --style=file $f; done
Move some characters around to optimize clang-format output.
Create a clang-format file, mark some regions with
// clang-format off
, and format the codebase.