[Lldb-commits] [PATCH] D153489: [lldb] Print hint if object description is requested but not implemented

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 marked an inline comment as done.
augusto2112 added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:133-135
+          << "note: object description requested, but type doesn't implement "
+             "a custom object description. Consider using \"p\" instead of "
+             "\"po\"\n";
----------------
kastiglione wrote:
> ok, I have a new suggestion. Since lldb will warn only once per target, and not per type, I think this note should be reworded to focus the guidance on the format of the output, not the type.
> 
> My concern is lldb emits basically "this type doesn't need a `po`", but then the diagnostic is printed for only one type, and never tells you about other types. How will people know that other types should use `p` not `po`?
> 
> If the message were on the format, and not the type, then I think it makes more sense as a once per target message.
> 
> A possible rewording:
> > note: this `po` used the default object description, which shows none of the objects properties. When you output like this, consider using `p` instead of `po` when you see such output.
Personally I find the new message a bit more confusing for users to understand. Maybe: 

```
note: object description requested, but type doesn't implement a custom object description. Consider using "p" instead of  "po" (this warning will only be displayed once per debug session).
```

What do you think?


================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:158-162
+        StreamString temp_result_stream;
+        valobj_sp->Dump(temp_result_stream, dump_options);
+        llvm::StringRef output = temp_result_stream.GetString();
+        maybe_add_hint(output);
+        result.GetOutputStream() << output;
----------------
kastiglione wrote:
> augusto2112 wrote:
> > kastiglione wrote:
> > > what do you think of passing in the `result`'s stream into `maybe_add_hint`? Perhaps I am overlooking something, but I wonder if it would simplify the code to reuse the one stream, instead of separating and then combining two streams.
> > I need the two streams to print it in the correct order (hint first, result later)
> do we have a precedent for before vs after? Maybe I need to see some examples, but I think it should be after. My logic is "here's the output you requested, and then here's a note about it". Also the note would be next to the next prompt, so maybe closer to the eyes? Just figured it was worth hashing out.
DWIM print will add the note beforehand, I don't have strong feelings about this either way though. We'd probably still need 2 streams though, since we only want to match what's added by the value object's `Dump`, and nothing that may already be on the stream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153489