JDevlieghere added a comment. I'm supportive of this effort, but if we're removing the plaintext format, I think it's worth sending this out as an RFC.
In D107079#2913760 <https://reviews.llvm.org/D107079#2913760>, @teemperor wrote: > Some general notes: > > - Why JSON? In LLVM we have pretty much just JSON and YAML as formats > available (well, and the bitcode format I guess). YAML is a much more > complicated format and the main idea here is to let people write tools, so > JSON seemed like an obvious pick. The YAML I/O library is also relatively slow, which would hurt the overhead of enabling logging (even more). > - Will there be an upstream log viewer? I don't know, but we could easily put > a simple one on the website that just formats a log as a simple HTML table. > That is easily done in vanilla JavaScript. But that should be a follow up > patch. Personally I'd also want a script to view the logs in the terminal. If we write it in Python we can ship it with lldb. > - Do we want to keep the old plain text format too? To be honest, I don't > really see much Me neither on the condition that users can convert the logs to some kind of plain text format (part of the motivation for my previous comment.) > - What about logs that are printed to stdin and we want to parse? I think you > can easily make a tool that filters out the non-JSON output. Logs are > guaranteed to have every object on its own line, so you can just go > line-by-line and filter them based on that. > > Some planned follow ups: > > - Enable more logging meta-information by default. Things such as pid/tid > seems like obvious cheap choices. +1, if the data is structured we should always emit it (unless there's some large associated cost) > - For people that really like the old format, it should be a simple patch to > add a converter that just takes a JSON file and outputs the old plaintext. > > - I want to remove the ability to turn off thread-safe logging. It seems like > it could potentially crash LLDB but it would certainly break the JSON parser. > It's usually (and luckily) enabled by default anyway. > > - Some more meta information would be actually helpful sometimes and there > isn't any reason to not add them. E.g., classifying messages as > error/warning/info. +1 on everything here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107079/new/ https://reviews.llvm.org/D107079 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
