+1 Definitely agree it improves readability, though it's gonna make for hellish PRs, so we should probably not go making this change all at once but instead do it as we happen to be working in files for things. That's my opinion at least.
--Matt On Wed, Aug 28, 2024, 12:02 AM Ruoxi Sun <zanmato1...@gmail.com> wrote: > +1. > > I think this definitely improves the readability. People no longer need to > pair the mind-blowing deep-nested #if/#endif-s. > > Thanks for doing this! > > *Regards,* > *Rossi SUN* > > > On Wed, Aug 28, 2024 at 9:07 AM Sutou Kouhei <k...@clear-code.com> wrote: > > > Hi, > > > > How about indenting preprocessor directives for readability? > > > > Issue: https://github.com/apache/arrow/issues/43796 > > PR: https://github.com/apache/arrow/pull/43798 > > > > For example: > > > > Before: > > > > ---- > > #ifndef NDEBUG > > #include "arrow/acero/options_internal.h" > > #endif > > ---- > > > > After: > > > > ---- > > #ifndef NDEBUG > > # include "arrow/acero/options_internal.h" > > #endif > > ---- > > > > > > FYI: Google C++ style guide (our coding style is based on > > it) doesn't require indent in preprocessor directives nor > > deny it: > > > > > https://google.github.io/styleguide/cppguide.html#Preprocessor_Directives > > > > ---- > > // Good - directives at beginning of line > > if (lopsided_score) { > > #if DISASTER_PENDING // Correct -- Starts at beginning of line > > DropEverything(); > > # if NOTIFY // OK but not required -- Spaces after # > > NotifyClient(); > > # endif > > #endif > > BackToNormal(); > > } > > ---- > > > > > > Thanks, > > -- > > kou > > >
