[libcxx-commits] [PATCH] D103368: [libc++][format] Adds parser std-format-spec.

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added inline comments.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:132
+  // character. Validate what fmt and MSVC have implemented.
+  _CharT __fill{_CharT(' ')};
+
----------------
vitaut wrote:
> Mordante wrote:
> > vitaut wrote:
> > > This should really be a grapheme cluster but {fmt} uses a code point at the moment (grapheme clusterization will be added later). In any case this should be a range (e.g. string_view) and not a single code unit.
> > My interpretation of http://eel.is/c++draft/format#string.std-2
> >   The fill character can be any character other than { or }.
> > is that this is intended to be a single character and not a Unicode grapheme cluster. I can't find a paper or defect report that intends to change the current behaviour. Was it intended to be part of P1868R2 "🦄 width: clarifying units of width and precision in std::format" ?
> > 
> > Is there a paper or defect report, or should I file a defect report?
> > 
> > I don't mind changing it, but for now there are some unclarities how to implement this feature:
> > Are 2 column Unicode characters allowed?
> > - if yes, how to do odd even formatting?
> > - if no, then is it a precondition or should the implementation do an width estimate?
> > 
> > For now I'll leave it as is, there's a `TODO FMT` to address this issue later.
> > should I file a defect report?
> 
> Please do open a LWG issue. The "character" term is ambiguous but if interpreted in the C++ sense the current wording is clearly wrong because you cannot even use box drawing characters (in the Unicode sense). We can discuss the details in the LWG issue.
So have you or do you plan to open an LWG issue? Otherwise I can do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103368