Message.ToString() does not indent submessages properly.

[GoogleCodeExporter]

What steps will reproduce the problem?

            var p1 = new Person.Builder
            {
                Name = "John Smith",
                Email = "johnsmith@email.com",
                Id = 22
            };
            p1.AddPhone(new Person.Types.PhoneNumber.Builder { Number = "2345678" });
            Console.WriteLine(p1.Build().ToString());

It prints:

name: "John Smith"
id: 22
email: "johnsmith@email.com"
phone {
number: "2345678"
}

What is the expected output? What do you see instead?

name: "John Smith"
id: 22
email: "johnsmith@email.com"
phone {
  number: "2345678"
}

*** Notice 2 trailing spaces before "number:" field.

Original issue reported on code.google.com by igorga...@gmail.com on 21 Feb 2014 at 11:06

[GoogleCodeExporter]

I meant leading spaces.

Original comment by igorga...@gmail.com on 21 Feb 2014 at 11:14

[GoogleCodeExporter]

(Sorry not to have replied before.)

I've just tried this myself, and it works fine for me. I copied and pasted your 
exact code. Are you still able to reproduce this? Which operating system is 
this running on?

Original comment by jonathan.skeet on 5 Apr 2014 at 3:11

[GoogleCodeExporter]

I need to double check that then. Could it be because of lite vs non-lite 
runtime?

Original comment by igorga...@gmail.com on 7 Apr 2014 at 3:24

[GoogleCodeExporter]

Hmm... possibly. TextFormat isn't available in the lite runtime. I need to get 
my head back into the lite runtime, and will then investigate. 

Original comment by jonathan.skeet on 7 Apr 2014 at 5:42

[GoogleCodeExporter]

Right, just verified - it does indeed do that with the lite runtime. Will fix 
(assuming it doesn't prove fiendish...)

Original comment by jonathan.skeet on 7 Apr 2014 at 6:21

• Changed state: Accepted

[GoogleCodeExporter]

It feels like you need a ToString(TextGenerator). And ToString() would look 
like:

string ToString() {
  var buffer = new StringWriter();  // Or equivalent.
  ToString(new TextGenerator(buffer));
  return buffer.ToString();
}

Original comment by igorga...@gmail.com on 7 Apr 2014 at 6:42

[GoogleCodeExporter]

Just to make this discussion a bit more interesting: Perhaps ToString() should 
not indent submessages, both in lite and non-lite runtimes.

In C++ world, protobuf objects have DebugString(), which indent submessages, 
and ShortDebugString() which prints everything in a single line.

While debugging in VS, it feels like ShortDebugString() is more appropriate 
since it is compact and no UI real state is wasted with indentation 
whitespaces. Therefore, ToString() should perhaps be a single compact line.

OTOH, indented output is far more useful everywhere else. But for all of them, 
user can call ToDebugString() instead of ToString().

That being said, if you are willing to have indented and non-indented versions, 
I'd have ToString() for short, single-lined output and ToDebugString() for 
fully indented output.

Original comment by igorga...@gmail.com on 7 Apr 2014 at 6:48

[GoogleCodeExporter]

The problem is that TextGenerator isn't part of the Lite runtime. If the aim of 
the Lite runtime is to really be as small as possible, I'm not sure it's worth 
adding TextGenerator. If we can get away without adding anything on a per-type 
basis, it might be okay... I'll give it a try.

I've just checked the Java version, and that doesn't override toString() at 
all, apparently.

Original comment by jonathan.skeet on 7 Apr 2014 at 6:49

[GoogleCodeExporter]

I definitely don't want to change the existing behaviour of full-fat ToString 
now - especially as it follows the behaviour of the Java code. I haven't used 
C++ protobufs, but I'm reasonably happy with the behaviour of the Java library 
:)

I'll see what I can do about the lite version though...

Original comment by jonathan.skeet on 7 Apr 2014 at 6:52

[GoogleCodeExporter]

Hmm.

Well it's *doable*:
https://code.google.com/p/protobuf-csharp-port/source/detail?r=3403e8a1b18170e7e
6c35896a94dbb561bfab5df&name=Issue74

(New branch for this issue.)

It feels incredibly ugly though. Given the *relative* rarity of the lite 
runtime (in my experience) and the relatively small inconvenience here, I think 
I'd probably rather just not fix this.

Thoughts? (Have a look at the change in the branch. No tests yet, but I've 
tested locally with the sample in the issue. It might fail for more deeply 
nested messages.)

Original comment by jonathan.skeet on 7 Apr 2014 at 7:30

[GoogleCodeExporter]

I think that the fix is fine. Although not fixing is certainly an option (which 
basically means: not implemented unless there is a demand for it). Perhaps 
leaving this issue open for votes/stars is a good thermometer.

Original comment by igorga...@gmail.com on 7 Apr 2014 at 7:40

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Yup, let's leave it unfixed for the moment - the branch will still be there 
later :)

(I had added another comment suggesting a horrible hack, but it would have been 
even more horrible than I thought, due to having to work out exactly when to 
indent.)

Original comment by jonathan.skeet on 7 Apr 2014 at 7:47

[GoogleCodeExporter]

Closing it as won't fix for now - I don't want to fix it for 2.x; if it works 
for 3.x then that's fine by me, but I don't think it'll be worth going to 
extraordinary measures for.

Original comment by jonathan.skeet on 15 Feb 2015 at 5:13

• Changed state: WontFix