argparse icon indicating copy to clipboard operation
argparse copied to clipboard

Inconsistency in Styling API

Open SirNickolas opened this issue 7 months ago • 2 comments

auto myBold = bold;
assert(is(typeof(myBold("text")) == string));
assert(!is(typeof(bold("text")): string)); // Not even convertible.

(That’s because bold()("text") returns string while bold("text") returns StyledText.)

IMHO, it shouldn’t work like this. Unfortunately, any change in this would be an API change. Is it justified for 2.0?

In case we decide to unify them, we’ll have at least 3 ways:

  1. Make everything return string, remove StyledText altogether.
  2. Make everything return StyledText, which will be implicitly convertible to string.
  3. Make everything return StyledText, which will retain its explicit toString method.

Why do we consider options 2 and 3? Because StyledText can become a lazy range – and it’ll be possible to rule out allocations of temporary strings:

// Allocates a temporary just to concatenate it; then it immediately becomes garbage.
string msg0 = "argument " ~ bold("name") ~ " is bold";

// Proposed alternative:
auto app = appender!string();
app ~= "argument ";
app ~= bold("name"); // No temporaries; writes directly into the buffer.
app ~= " is bold";
string msg1 = app[];

// The same in one line:
string msg2 = chain("argument ".byCodeUnit, bold("name"), " is bold".byCodeUnit).array();

I think StyledText can retain its opBinary!"~" and opBinaryRight!"~" for convenience: they clearly look like something that allocates. So all the snippets above will be compilable.

The question is, whether or not StyledText should implicitly convert to string. To be honest, a lazy range that can implicitly lose its laziness scares me off. I’d prefer option 3.

Implications:

  1. string msg = bold("text").toString() will continue to work.
  2. app ~= config.programName("text") will continue to work.
  3. writeln(config.programName("text")) will continue to work: writeln invokes toString.
  4. string msg = config.programName("text") ~ " is bold" will continue to work.
  5. string msg = config.programName("text") will break with approach 3: an explicit toString is required, just like in example 1 above.

SirNickolas avatar Dec 01 '23 15:12 SirNickolas

I tried to optimize generation of styled text (to postpone adding ANSI sequences until toString). Maybe it was over engineering. I'll try to make it simpler.

andrey-zherikov avatar Jan 14 '24 01:01 andrey-zherikov

5. string msg = config.programName("text") will break with approach 3: an explicit toString is required, just like in example 1 above.

Why are you saying that it "will break"? It doesn't compile even now so I see no problem here:

Error: cannot implicitly convert expression StyleImpl("text") of type StyledText to string

andrey-zherikov avatar Jan 17 '24 20:01 andrey-zherikov