[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa69f78b080ef: [lldb] Add syntax color highlighting for disassembly (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D159164?vs=555489&id=13

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/source/Core/Disassembler.cpp:662-663 // consistent column spacing in these cases, unfortunately. - if (m_opcode_name.length() >= opcode_column_width) { -opcode_column_width = m_opcode_name.length() + 1; + if (opcode_name.l

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Thanks Greg! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 ___ lldb-commits mailing list lldb-commits@lists.llvm.org htt

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Just one last inline comment where we can't use the colorized string to calculate the column width and this is good to go. Comment at: lldb/source/Core/Disassembler.cpp:662-663 // consistent column spacing in these cases, unfortunately. - if (m_op

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 555489. JDevlieghere marked 9 inline comments as done. JDevlieghere added a comment. Use the execution context to enable colors. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 Files: lldb/include/l

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/include/lldb/Core/Disassembler.h:158 bool show_bytes, bool show_control_flow_kind, -const ExecutionContext *exe_ctx, +bool show_color, const ExecutionContext *exe_ctx,

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. @clayborg let me know if you're happy with this. To answer your previous question, I'm not planning anymore changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 ___

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 555442. JDevlieghere added a comment. As I was adding another test I realized an issue with the previous approach due to the fact that we cache the result. The solution is to cache both the plain and the marked up result, and have the different accessor

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. lgtm. Is there any more things you want to add to this patch test wise, or is this complete? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 ___ lldb-commits mailing list l

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 555417. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Test `GetMnemonic` and `GetComment`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 Files: lldb/include/lldb/Cor

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Yes, this simplifies the patch a lot. Looks good after we check the mnemonic and comment for no color when used through the SBInstruction APIs. Comment at: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py:69 +ci.Han

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-31 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 555269. JDevlieghere added a comment. Discard whitespace changes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 Files: lldb/include/lldb/Core/Disassembler.h lldb/source/Core/Disassembler.cpp ll

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-31 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 555268. JDevlieghere added a comment. Address @clayborg's code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 Files: lldb/include/lldb/Core/Disassembler.h lldb/source/Core/Disasse

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-31 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked 6 inline comments as done. JDevlieghere added a comment. In D159164#4632017 , @clayborg wrote: > We shouldn't need to pass in "bool use_color" to the Disassembler creation > functions as the only reason to pass something to the creati

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-31 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. We shouldn't need to pass in "bool use_color" to the Disassembler creation functions as the only reason to pass something to the creation function for any plug-in is if that data would exclude a disassembler if it didn't support color. But we always want to see some di

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 554873. JDevlieghere added a comment. - Fix inconsistencies and use `use_color` everywhere CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159164/new/ https://reviews.llvm.org/D159164 Files: lldb/include/lldb/Core/Disassembler.h lldb/include

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-30 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. LGTM this is just mechanical passing around of the user's setting though the layers. Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h:76

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-29 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. Similar to the LLVM patch this depends on, this patch is a WIP to get some early feedback. This definitely needs a test. Most of the patch deals with plumbing the value of `Debugger::GetUseColor` to the `Disassembler`. CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-29 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision. JDevlieghere added reviewers: jasonmolenda, DavidSpickett, clayborg. Herald added subscribers: jrtc27, sdardis. Herald added a project: All. JDevlieghere requested review of this revision. Add support for syntax highlighting assembly. The current patch uses a d