[libcxx-commits] [PATCH] D128577: [libc++][chrono] Implements formatter day.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 5 inline comments as done.
Mordante added inline comments.


================
Comment at: libcxx/include/__chrono/formatter.h:60-61
+  tm __result{
+      .tm_sec   = 0,
+      .tm_min   = 0,
+      .tm_hour  = 0,
----------------
philnik wrote:
> Mordante wrote:
> > philnik wrote:
> > > Mordante wrote:
> > > > ldionne wrote:
> > > > > Suggestion:
> > > > > 
> > > > > ```
> > > > >   .tm_sec = 0
> > > > > , .tm_min = 0
> > > > > , .tm_hour = 0
> > > > > ...
> > > > > #ifdef __GLIBC__
> > > > > , .tm_gmtoff = 0
> > > > > , .tm_zone = "UTC"
> > > > > #endif
> > > > > ```
> > > > > 
> > > > > IDK if clang-format will handle that gracefully.
> > > > I haven't found a clang-format option to do this. Only for the constructor. But I agree it would be great when clang-format would have an option to do this.
> > > What's wrong with just using a trailing comma?
> > In general nothing, but line 62 now has a single comma. It would be really nice of clang-format could detect these cases an used leading commas instead. IMO that would be nice to have, but I've no objection against the single comma.
> I meant "Why not put the comma at the end of line 60?"
> Another thought: Why do you explicitly initialize all the struct members? They get value-initialized anyways if you omit them. That would leave you with just
> ```
> tm __result {
> #ifdef __GLIBC__
>   .tm_zone = "UTC",
> #endif
> };
> ```
> 
@vitaut suggested something similar. I like this approach better, modulo the trailing comma.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128577/new/

https://reviews.llvm.org/D128577