1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 19:27:45 +00:00

Spreadsheet+LibSyntax: Never insert spans directly

Function `CellSyntaxHighlighter::rehighlight()` direct inserted spans
to TextDocument `m_span` vector missing out important reordering and
merging operation carried out by `TextDocument::set_spans()`.

This caused overlapping spans for a cell with only a `=` symbol
(one for the actual token and one for the highlighting) to
miscalculate `start` and `end` value and a `length` value (of
`size_t` type) with a `0-1` substraction (result: 18446744073709551615)
causing `Utf32View::substring_view()` to fail the
`!Checked<size_t>::addition_would_overflow(offset, length)` assertion

This remove the possibility to directly alter `TextDocument`'s spans
thus forcing the utilization of `HighlighterClient::do_set_spans()`
interface function.

Proper refactor have been applied to
`CellSyntaxHighlighter::rehighlight()` function
This commit is contained in:
Matteo Benetti 2023-04-13 14:44:41 +02:00 committed by Andreas Kling
parent fb47b988ca
commit 3e1626acdc
4 changed files with 31 additions and 30 deletions

View file

@ -1,5 +1,6 @@
/* /*
* Copyright (c) 2020-2022, the SerenityOS developers. * Copyright (c) 2020-2022, the SerenityOS developers.
* Copyright (c) 2023, Matteo benetti <matteo.benetti@proton.me>
* *
* SPDX-License-Identifier: BSD-2-Clause * SPDX-License-Identifier: BSD-2-Clause
*/ */
@ -13,17 +14,16 @@ namespace Spreadsheet {
void CellSyntaxHighlighter::rehighlight(Palette const& palette) void CellSyntaxHighlighter::rehighlight(Palette const& palette)
{ {
auto text = m_client->get_text(); m_client->clear_spans();
m_client->spans().clear();
if (!text.starts_with('=')) { if (m_client->get_text().starts_with('=')) {
m_client->do_update();
return;
}
JS::SyntaxHighlighter::rehighlight(palette); JS::SyntaxHighlighter::rehighlight(palette);
auto spans = m_client->spans();
// Highlight the '=' // Highlight the '='
m_client->spans().empend( spans.empend(
GUI::TextRange { { 0, 0 }, { 0, 1 } }, GUI::TextRange { { 0, 0 }, { 0, 1 } },
Gfx::TextAttributes { Gfx::TextAttributes {
palette.syntax_keyword(), palette.syntax_keyword(),
@ -36,21 +36,24 @@ void CellSyntaxHighlighter::rehighlight(Palette const& palette)
if (m_cell && m_cell->thrown_value().has_value()) { if (m_cell && m_cell->thrown_value().has_value()) {
if (auto value = m_cell->thrown_value().value(); value.is_object() && is<JS::Error>(value.as_object())) { if (auto value = m_cell->thrown_value().value(); value.is_object() && is<JS::Error>(value.as_object())) {
auto& error = static_cast<JS::Error const&>(value.as_object()); auto& error = static_cast<JS::Error const&>(value.as_object());
auto& traceback = error.traceback(); auto& range = error.traceback().first().source_range;
auto& range = traceback.first().source_range;
GUI::TextRange text_range { { range.start.line - 1, range.start.column }, { range.end.line - 1, range.end.column } }; spans.prepend({
m_client->spans().prepend( GUI::TextRange { { range.start.line - 1, range.start.column }, { range.end.line - 1, range.end.column } },
GUI::TextDocumentSpan {
text_range,
Gfx::TextAttributes { Gfx::TextAttributes {
Color::Black, Color::Black,
Color::Red, Color::Red,
false, false,
}, },
(u64)-1, (u64)-1,
false }); false,
});
} }
} }
m_client->do_set_spans(move(spans));
}
m_client->do_update(); m_client->do_update();
} }
} }

View file

@ -306,7 +306,6 @@ private:
virtual void document_did_update_undo_stack() override; virtual void document_did_update_undo_stack() override;
// ^Syntax::HighlighterClient // ^Syntax::HighlighterClient
virtual Vector<TextDocumentSpan>& spans() final { return document().spans(); }
virtual Vector<TextDocumentSpan> const& spans() const final { return document().spans(); } virtual Vector<TextDocumentSpan> const& spans() const final { return document().spans(); }
virtual void set_span_at_index(size_t index, TextDocumentSpan span) final { document().set_span_at_index(index, move(span)); } virtual void set_span_at_index(size_t index, TextDocumentSpan span) final { document().set_span_at_index(index, move(span)); }
virtual Vector<GUI::TextDocumentFoldingRegion>& folding_regions() final { return document().folding_regions(); }; virtual Vector<GUI::TextDocumentFoldingRegion>& folding_regions() final { return document().folding_regions(); };

View file

@ -125,7 +125,6 @@ public:
} }
private: private:
virtual Vector<GUI::TextDocumentSpan>& spans() override { return m_spans; }
virtual Vector<GUI::TextDocumentSpan> const& spans() const override { return m_spans; } virtual Vector<GUI::TextDocumentSpan> const& spans() const override { return m_spans; }
virtual void set_span_at_index(size_t index, GUI::TextDocumentSpan span) override { m_spans.at(index) = move(span); } virtual void set_span_at_index(size_t index, GUI::TextDocumentSpan span) override { m_spans.at(index) = move(span); }

View file

@ -17,9 +17,9 @@ class HighlighterClient {
public: public:
virtual ~HighlighterClient() = default; virtual ~HighlighterClient() = default;
virtual Vector<GUI::TextDocumentSpan>& spans() = 0;
virtual Vector<GUI::TextDocumentSpan> const& spans() const = 0; virtual Vector<GUI::TextDocumentSpan> const& spans() const = 0;
virtual void set_span_at_index(size_t index, GUI::TextDocumentSpan span) = 0; virtual void set_span_at_index(size_t index, GUI::TextDocumentSpan span) = 0;
virtual void clear_spans() { do_set_spans({}); };
virtual Vector<GUI::TextDocumentFoldingRegion>& folding_regions() = 0; virtual Vector<GUI::TextDocumentFoldingRegion>& folding_regions() = 0;
virtual Vector<GUI::TextDocumentFoldingRegion> const& folding_regions() const = 0; virtual Vector<GUI::TextDocumentFoldingRegion> const& folding_regions() const = 0;