This revision was automatically updated to reflect the committed changes.
Closed by commit rL338662: [LLDB] Added syntax highlighting support (authored
by teemperor, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D49334?vs=157702&id=158671#toc
Repository:
rL LLVM
https://
teemperor marked an inline comment as done.
teemperor added inline comments.
Comment at: source/Core/Highlighter.cpp:34
+ // Calculate how many bytes we have written.
+ return m_prefix.size() + value.size() + m_suffix.size();
+}
labath wrote:
> teemperor wrote:
teemperor updated this revision to Diff 157702.
teemperor added a comment.
- Removed extra semicolons that slipped in somehow.
https://reviews.llvm.org/D49334
Files:
include/lldb/Core/Debugger.h
include/lldb/Core/Highlighter.h
include/lldb/Target/Language.h
packages/Python/lldbsuite/tes
labath added a comment.
Thanks for the explanation. This looks fine to me, though I would feel better
if someone else gave it a look too.
Comment at: source/Core/Highlighter.cpp:29
+ if (!m_prefix.empty())
+s << lldb_utility::ansi::FormatAnsiTerminalCodes(m_prefix);
+ s
teemperor added inline comments.
Comment at: source/Core/Highlighter.cpp:34
+ // Calculate how many bytes we have written.
+ return m_prefix.size() + value.size() + m_suffix.size();
+}
labath wrote:
> This isn't correct, as you're not writing m_prefix, but it's
teemperor updated this revision to Diff 157320.
teemperor marked 8 inline comments as done.
teemperor added a comment.
- Addressed Pavel's comments. Also cleaned up some more includes.
Yes, knowing the language we currently display would be nice, but I didn't see
a good way to reliably get this
labath added a comment.
The patch is bigger than ideal for a single change, but I like the way it is
structured. Thank you for abstracting the clang specifics.
The one high-level question that came to mind when looking this over is whether
we really need to do all this file name matching to get
teemperor updated this revision to Diff 157169.
teemperor added a reviewer: davide.
teemperor added a comment.
- Removed some leftover code from the refactoring.
- Correctly SetUp/TearDown the test case.
https://reviews.llvm.org/D49334
Files:
include/lldb/Core/Debugger.h
include/lldb/Core/H
teemperor planned changes to this revision.
teemperor added a comment.
Actually, we also have to tear down the plugins, so let me fix that first.
https://reviews.llvm.org/D49334
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.
teemperor updated this revision to Diff 157054.
teemperor added a comment.
[Updated patch to address Pavel's comments, thanks!]
@zturner So I looked into the Windows support:
Windows requires us to directly flush/signal/write/flush to the console output
stream. However lldb's output is buffered
teemperor added a comment.
@zturner We can migrate the existing AnsiTerminal.h to reuse the LLVM coloring
backend. This way we fix also the code that already uses this convenient
interface.
@labath I think we can add to the Language class the option to add its related
syntax highlighting suppo
labath added a comment.
We're also trying to avoid adding new clang-specific code to the debugger core.
I think it would make more sense if the (clang-based) c++ highlighter was
provided by some plugin. I see a couple of options:
- the c++ language plugin: I think this is the most low-level plu
zturner added a subscriber: teemperor.
zturner added a comment.
Ansi escape codes do not work on Windows. LLVM’s stream output classes have
color support that does work in a cross platform manner. Can we use those
instead?
Repository:
rL LLVM
https://reviews.llvm.org/D49334
___
13 matches
Mail list logo
