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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__chrono/formatter.h:60-61
+  tm __result{
+      .tm_sec   = 0,
+      .tm_min   = 0,
+      .tm_hour  = 0,
----------------
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
};
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128577