From 8d7fdeb633f365b2634ead9553ccfa278da6a4c6 Mon Sep 17 00:00:00 2001
From: Kyle Reed <kareed@kallanreed.com>
Date: Thu, 1 Jun 2023 15:45:55 -0700
Subject: [PATCH] Add edit support for Notepad (#1093)
* WIP file editing
* WIP file editing
* Add "on_pop" handler to navigation.
* WIP Editing
* WIP for draft
* Fix mock and unit tests, support +newline at end.
* Clean up Painter API and use string_view
* Fix optional rvalue functions
* Fix Result 'take' to be more standard
* FileWrapper stack buffer reads
* Grasping at straws
* Nit
* Move set_on_pop impl to cpp
* Workaround "Open" when file not dirty.
---------
Co-authored-by: kallanreed <kallanreed@outlook.com>
---
firmware/application/apps/ui_text_editor.cpp | 192 +++++++--
firmware/application/apps/ui_text_editor.hpp | 29 +-
firmware/application/file.cpp | 22 +-
firmware/application/file.hpp | 12 +-
firmware/application/file_wrapper.hpp | 217 ++++++++--
firmware/application/ui_navigation.cpp | 67 ++--
firmware/application/ui_navigation.hpp | 18 +-
firmware/common/optional.hpp | 6 +-
firmware/common/ui_painter.cpp | 36 +-
firmware/common/ui_painter.hpp | 24 +-
.../test/application/test_file_wrapper.cpp | 372 +++++++++++++++++-
11 files changed, 847 insertions(+), 148 deletions(-)
diff --git a/firmware/application/apps/ui_text_editor.cpp b/firmware/application/apps/ui_text_editor.cpp
index 1329e049..675d2e74 100644
--- a/firmware/application/apps/ui_text_editor.cpp
+++ b/firmware/application/apps/ui_text_editor.cpp
@@ -124,6 +124,22 @@ bool TextViewer::on_encoder(EncoderEvent delta) {
return updated;
}
+void TextViewer::redraw(bool redraw_text) {
+ paint_state_.redraw_text = redraw_text;
+ set_dirty();
+}
+
+uint32_t TextViewer::offset() const {
+ auto range = file_->line_range(cursor_.line);
+ if (range)
+ return range->start + col();
+ return 0;
+}
+
+uint16_t TextViewer::line_length() {
+ return file_->line_length(cursor_.line);
+}
+
bool TextViewer::apply_scrolling_constraints(int16_t delta_line, int16_t delta_col) {
if (!has_file())
return false;
@@ -175,28 +191,24 @@ bool TextViewer::apply_scrolling_constraints(int16_t delta_line, int16_t delta_c
return true;
}
-void TextViewer::redraw(bool redraw_text) {
- paint_state_.redraw_text = redraw_text;
- set_dirty();
-}
-
void TextViewer::paint_text(Painter& painter, uint32_t line, uint16_t col) {
auto r = screen_rect();
+ char buffer[max_col + 1];
// Draw the lines from the file
for (auto i = 0u; i < max_line; ++i) {
if (line + i >= file_->line_count())
break;
- auto str = file_->get_text(line + i, col, max_col);
+ auto result = file_->get_text(line + i, col, buffer, max_col);
- if (str && str->length() > 0)
+ if (result && *result > 0)
painter.draw_string(
{0, r.top() + (int)i * char_height},
- style_text, *str);
+ style_text, {buffer, *result});
// Clear empty line sections. This is less visually jarring than full clear.
- int32_t clear_width = max_col - (str ? str->length() : 0);
+ int32_t clear_width = max_col - (result ? *result : 0);
if (clear_width > 0)
painter.fill_rectangle(
{(max_col - clear_width) * char_width,
@@ -238,10 +250,6 @@ void TextViewer::reset_file(FileWrapper* file) {
redraw(true);
}
-uint16_t TextViewer::line_length() {
- return file_->line_length(cursor_.line);
-}
-
/* TextEditorMenu ***************************************************/
TextEditorMenu::TextEditorMenu()
@@ -309,25 +317,52 @@ TextEditorView::TextEditorView(NavigationView& nav)
menu.on_copy() = [this]() {
show_nyi();
};
+
menu.on_delete_line() = [this]() {
- show_nyi();
+ prepare_for_write();
+ file_->delete_line(viewer.line());
+ refresh_ui();
+ hide_menu(true);
};
+
menu.on_edit_line() = [this]() {
- show_nyi();
+ show_edit_line();
};
+
menu.on_add_line() = [this]() {
- show_nyi();
+ prepare_for_write();
+
+ if (viewer.offset() < file_->size() - 1)
+ file_->insert_line(viewer.line());
+ else
+ file_->insert_line(-1); // Add after last line.
+
+ refresh_ui();
+ hide_menu(true);
};
+
menu.on_open() = [this]() {
- // TODO: confirm.
- show_file_picker();
+ /*show_save_prompt([this]() {
+ show_file_picker();
+ });*/
+ // HACK: above should work but it's faulting.
+ if (!file_dirty_) {
+ show_file_picker();
+ } else {
+ show_save_prompt(nullptr);
+ show_file_picker(false);
+ }
};
+
menu.on_save() = [this]() {
- show_nyi();
+ save_temp_file();
+ hide_menu(true);
};
+
menu.on_exit() = [this]() {
- // TODO: confirm.
- nav_.pop();
+ show_save_prompt([this]() {
+ nav_.pop();
+ });
};
button_menu.on_select = [this]() {
@@ -345,6 +380,10 @@ TextEditorView::TextEditorView(NavigationView& nav, const fs::path& path)
open_file(path);
}
+TextEditorView::~TextEditorView() {
+ delete_temp_file();
+}
+
void TextEditorView::on_show() {
if (file_)
viewer.focus();
@@ -353,14 +392,21 @@ void TextEditorView::on_show() {
}
void TextEditorView::open_file(const fs::path& path) {
+ file_.reset();
+ viewer.clear_file();
+ delete_temp_file();
+
+ path_ = {};
+ file_dirty_ = false;
+ has_temp_file_ = false;
auto result = FileWrapper::open(path);
if (!result) {
nav_.display_modal("Read Error", "Cannot open file:\n" + result.error().what());
- file_.reset();
- viewer.clear_file();
+
} else {
- file_ = result.take();
+ file_ = *std::move(result);
+ path_ = path;
viewer.set_file(*file_);
}
@@ -401,16 +447,100 @@ void TextEditorView::hide_menu(bool hidden) {
set_dirty();
}
-void TextEditorView::show_file_picker() {
- auto open_view = nav_.push<FileLoadView>("");
- open_view->on_changed = [this](std::filesystem::path path) {
- open_file(path);
- hide_menu();
- };
+void TextEditorView::show_file_picker(bool immediate) {
+ // TODO: immediate is a hack until nav_.on_pop is fixed.
+ auto open_view = immediate ? nav_.push<FileLoadView>("") : nav_.push_under_current<FileLoadView>("");
+
+ if (open_view) {
+ open_view->on_changed = [this](std::filesystem::path path) {
+ open_file(path);
+ hide_menu();
+ };
+ }
+}
+
+void TextEditorView::show_edit_line() {
+ auto str = file_->get_text(viewer.line(), 0, viewer.line_length());
+ if (!str) {
+ nav_.display_modal("Error", "Failed to get line text.");
+ return;
+ }
+
+ edit_line_buffer_ = *std::move(str);
+
+ text_prompt(
+ nav_,
+ edit_line_buffer_,
+ viewer.col(),
+ max_edit_length,
+ [this](std::string& buffer) {
+ auto range = file_->line_range(viewer.line());
+ if (!range)
+ return;
+
+ prepare_for_write();
+ file_->replace_range(*range, buffer);
+ });
+ nav_.set_on_pop([this]() {
+ edit_line_buffer_.clear();
+ refresh_ui();
+ hide_menu(true);
+ });
}
void TextEditorView::show_nyi() {
nav_.display_modal("Soon...", "Coming soon.");
}
-} // namespace ui
\ No newline at end of file
+void TextEditorView::show_save_prompt(std::function<void()> continuation) {
+ if (!file_dirty_) {
+ if (continuation)
+ continuation();
+ return;
+ }
+
+ nav_.display_modal(
+ "Save?", "Save changes?", YESNO,
+ [this](bool choice) {
+ if (choice)
+ save_temp_file();
+ });
+ nav_.set_on_pop(continuation);
+}
+
+void TextEditorView::prepare_for_write() {
+ file_dirty_ = true;
+
+ if (has_temp_file_)
+ return;
+
+ // Copy to temp file on write.
+ has_temp_file_ = true;
+ delete_temp_file();
+ copy_file(path_, get_temp_path());
+ file_->assume_file(get_temp_path());
+}
+
+void TextEditorView::delete_temp_file() const {
+ auto temp_path = get_temp_path();
+ if (!temp_path.empty()) {
+ delete_file(temp_path);
+ }
+}
+
+void TextEditorView::save_temp_file() {
+ if (file_dirty_) {
+ delete_file(path_);
+ copy_file(get_temp_path(), path_);
+ file_dirty_ = false;
+ }
+}
+
+fs::path TextEditorView::get_temp_path() const {
+ if (!path_.empty())
+ return path_ + "~";
+
+ return {};
+}
+
+} // namespace ui
diff --git a/firmware/application/apps/ui_text_editor.hpp b/firmware/application/apps/ui_text_editor.hpp
index 9f4ac65a..2314af05 100644
--- a/firmware/application/apps/ui_text_editor.hpp
+++ b/firmware/application/apps/ui_text_editor.hpp
@@ -19,6 +19,10 @@
* Boston, MA 02110-1301, USA.
*/
+/* TODO:
+ * - Busy indicator while reading files.
+ */
+
#ifndef __UI_TEXT_EDITOR_H__
#define __UI_TEXT_EDITOR_H__
@@ -66,6 +70,10 @@ class TextViewer : public Widget {
uint32_t line() const { return cursor_.line; }
uint32_t col() const { return cursor_.col; }
+ uint32_t offset() const;
+
+ // Gets the length of the current line.
+ uint16_t line_length();
private:
static constexpr int8_t char_width = 5;
@@ -89,9 +97,6 @@ class TextViewer : public Widget {
void reset_file(FileWrapper* file = nullptr);
- // Gets the length of the current line.
- uint16_t line_length();
-
FileWrapper* file_{};
struct {
@@ -201,6 +206,7 @@ class TextEditorView : public View {
TextEditorView(
NavigationView& nav,
const std::filesystem::path& path);
+ ~TextEditorView();
std::string title() const override {
return "Notepad";
@@ -209,15 +215,30 @@ class TextEditorView : public View {
void on_show() override;
private:
+ static constexpr size_t max_edit_length = 1024;
+ std::string edit_line_buffer_{};
+
void open_file(const std::filesystem::path& path);
+ void save_file();
void refresh_ui();
void update_position();
void hide_menu(bool hidden = true);
- void show_file_picker();
+ void show_file_picker(bool immediate = true);
+ void show_edit_line();
void show_nyi();
+ void show_save_prompt(std::function<void()> continuation);
+
+ void prepare_for_write();
+ void create_temp_file() const;
+ void delete_temp_file() const;
+ void save_temp_file();
+ std::filesystem::path get_temp_path() const;
NavigationView& nav_;
std::unique_ptr<FileWrapper> file_{};
+ std::filesystem::path path_{};
+ bool file_dirty_{false};
+ bool has_temp_file_{false};
TextViewer viewer{
/* 272 = 320 - 16 (top bar) - 32 (bottom controls) */
diff --git a/firmware/application/file.cpp b/firmware/application/file.cpp
index dbbeabd7..ea7bdb33 100644
--- a/firmware/application/file.cpp
+++ b/firmware/application/file.cpp
@@ -44,8 +44,9 @@ Optional<File::Error> File::open_fatfs(const std::filesystem::path& filename, BY
}
}
-Optional<File::Error> File::open(const std::filesystem::path& filename) {
- return open_fatfs(filename, FA_READ);
+Optional<File::Error> File::open(const std::filesystem::path& filename, bool read_only) {
+ BYTE mode = read_only ? FA_READ : FA_READ | FA_WRITE;
+ return open_fatfs(filename, mode);
}
Optional<File::Error> File::append(const std::filesystem::path& filename) {
@@ -97,6 +98,15 @@ File::Result<File::Offset> File::seek(Offset new_position) {
return {static_cast<File::Offset>(old_position)};
}
+File::Result<File::Offset> File::truncate() {
+ const auto position = f_tell(&f);
+ const auto result = f_truncate(&f);
+ if (result != FR_OK) {
+ return {static_cast<Error>(result)};
+ }
+ return {static_cast<File::Offset>(position)};
+}
+
File::Size File::size() const {
return f_size(&f);
}
@@ -248,19 +258,19 @@ std::filesystem::filesystem_error copy_file(
File dst;
auto error = src.open(file_path);
- if (error.is_valid()) return error.value();
+ if (error) return error.value();
error = dst.create(dest_path);
- if (error.is_valid()) return error.value();
+ if (error) return error.value();
while (true) {
auto result = src.read(buffer, buffer_size);
if (result.is_error()) return result.error();
- result = dst.write(buffer, result.value());
+ result = dst.write(buffer, *result);
if (result.is_error()) return result.error();
- if (result.value() < buffer_size)
+ if (*result < buffer_size)
break;
}
diff --git a/firmware/application/file.hpp b/firmware/application/file.hpp
index 0245cd7e..a93d5492 100644
--- a/firmware/application/file.hpp
+++ b/firmware/application/file.hpp
@@ -320,13 +320,8 @@ class File {
return value_;
}
- /* Allows value to be moved out of the Result. */
- T take() {
- if (is_error())
- return {};
- T temp;
- std::swap(temp, value_);
- return temp;
+ T&& operator*() && {
+ return std::move(value_);
}
Error error() const {
@@ -369,7 +364,7 @@ class File {
File& operator=(const File&) = delete;
// TODO: Return Result<>.
- Optional<Error> open(const std::filesystem::path& filename);
+ Optional<Error> open(const std::filesystem::path& filename, bool read_only = true);
Optional<Error> append(const std::filesystem::path& filename);
Optional<Error> create(const std::filesystem::path& filename);
@@ -377,6 +372,7 @@ class File {
Result<Size> write(const void* data, Size bytes_to_write);
Result<Offset> seek(uint64_t Offset);
+ Result<Offset> truncate();
// Timestamp created_date() const;
Size size() const;
diff --git a/firmware/application/file_wrapper.hpp b/firmware/application/file_wrapper.hpp
index 13db3621..73a8a6d6 100644
--- a/firmware/application/file_wrapper.hpp
+++ b/firmware/application/file_wrapper.hpp
@@ -27,7 +27,7 @@
#include "optional.hpp"
#include <memory>
-#include <string>
+#include <string_view>
enum class LineEnding : uint8_t {
LF,
@@ -36,12 +36,20 @@ enum class LineEnding : uint8_t {
/* TODO:
* - CRLF handling.
+ * - Avoid full re-read on edits.
+ * - Would need to read old/new text when editing to track newlines.
+ * - How to surface errors? Exceptions?
*/
+/* FatFs docs http://elm-chan.org/fsw/ff/00index_e.html */
+
/* BufferType requires the following members
* Size size()
* Result<Size> read(void* data, Size bytes_to_read)
+ * Result<Size> write(const void* data, Size bytes_to_write)
* Result<Offset> seek(uint32_t offset)
+ * Result<Offset> truncate()
+ * Optional<Error> sync()
*/
/* Wraps a buffer and provides an API for accessing lines efficiently. */
@@ -52,10 +60,12 @@ class BufferWrapper {
using Line = uint32_t;
using Column = uint32_t;
using Range = struct {
- // Offset of the line start.
+ // Offset of the start, inclusive.
Offset start;
- // Offset of one past the line end.
+ // Offset of the end, exclusive.
Offset end;
+
+ Offset length() const { return end - start; }
};
BufferWrapper(BufferType* buffer)
@@ -69,6 +79,18 @@ class BufferWrapper {
BufferWrapper& operator=(const BufferWrapper&) = delete;
Optional<std::string> get_text(Line line, Column col, Offset length) {
+ std::string buffer;
+ buffer.resize(length);
+
+ auto result = get_text(line, col, &buffer[0], length);
+ if (!result)
+ return {};
+
+ buffer.resize(*result);
+ return buffer;
+ }
+
+ Optional<Offset> get_text(Line line, Column col, char* output, Offset length) {
auto range = line_range(line);
int32_t to_read = length;
@@ -82,7 +104,7 @@ class BufferWrapper {
if (to_read <= 0)
return {};
- return read(range->start + col, to_read);
+ return read(range->start + col, output, to_read);
}
/* Gets the size of the buffer in bytes. */
@@ -95,12 +117,12 @@ class BufferWrapper {
Optional<Range> line_range(Line line) {
ensure_cached(line);
- auto offset = offset_for_line(line);
- if (!offset)
+ auto index = index_for_line(line);
+ if (!index)
return {};
- auto start = *offset == 0 ? start_offset_ : (newlines_[*offset - 1] + 1);
- auto end = newlines_[*offset] + 1;
+ auto start = *index == 0 ? start_offset_ : (newlines_[*index - 1] + 1);
+ auto end = newlines_[*index] + 1;
return Range{start, end};
}
@@ -108,17 +130,66 @@ class BufferWrapper {
/* Gets the length of the line, or 0 if invalid. */
Offset line_length(Line line) {
auto range = line_range(line);
+ return range ? range->length() : 0;
+ }
+
+ /* Gets the buffer offset of the line & col if valid. */
+ Optional<Offset> get_offset(Line line, Column col) {
+ auto range = line_range(line);
if (range)
- return range->end - range->start;
+ return range->start + col;
- return 0;
+ return {};
}
/* Gets the index of the first line in the cache.
* Only really useful for unit testing or diagnostics. */
Offset start_line() { return start_line_; };
+ /* Inserts a line before the specified line or at the
+ * end of the buffer if line >= line_count. */
+ void insert_line(Line line) {
+ auto range = line_range(line);
+
+ if (range)
+ replace_range({range->start, range->start}, "\n");
+ else if (line >= line_count_)
+ replace_range({(Offset)size(), (Offset)size()}, "\n");
+ }
+
+ /* Deletes the specified line. */
+ void delete_line(Line line) {
+ auto range = line_range(line);
+
+ if (range)
+ replace_range(*range, {});
+ }
+
+ /* Replace the specified range with the string contents.
+ * A range with start/end set to the same value will insert.
+ * A range with an empty string will delete. */
+ void replace_range(Range range, std::string_view value) {
+ if (range.start > size() || range.end > size() || range.start > range.end)
+ return;
+
+ /* If delta_length == 0, it's an overwrite. Could still have
+ * added or removed newlines so caches will need to be rebuilt.
+ * If delta_length > 0, the file needs to grow and content needs
+ * to be shifted forward until the end of the range.
+ * If delta_length < 0, the file needs to be truncated and the
+ * content after the value needs to be shifted backward. */
+ int32_t delta_length = value.length() - range.length();
+ if (delta_length > 0)
+ expand(range.end, delta_length);
+ else if (delta_length < 0)
+ shrink(range.end, delta_length);
+
+ write(range.start, value);
+ wrapped_->sync();
+ rebuild_cache();
+ }
+
protected:
BufferWrapper() {}
@@ -132,12 +203,16 @@ class BufferWrapper {
static constexpr Offset max_newlines = CacheSize;
/* Size of stack buffer used for reading/writing. */
- static constexpr size_t buffer_size = 512;
+ static constexpr Offset buffer_size = 512;
void initialize() {
start_offset_ = 0;
start_line_ = 0;
line_count_ = 0;
+ rebuild_cache();
+ }
+
+ void rebuild_cache() {
newlines_.clear();
// Special case for empty files to keep them consistent.
@@ -147,7 +222,17 @@ class BufferWrapper {
return;
}
- Offset offset = 0;
+ // TODO: think through this for edit cases.
+ // E.g. don't read to end, maybe could specify
+ // a range to re-read because it should be possible
+ // to tell where the dirty regions are. After the
+ // dirty region, it should be possible to fixup
+ // the line_count data.
+ // TODO: seems like shrink/expand could do this while
+ // they are running.
+
+ line_count_ = start_line_;
+ Offset offset = start_offset_;
auto result = next_newline(offset);
while (result) {
@@ -159,25 +244,28 @@ class BufferWrapper {
}
}
- Optional<std::string> read(Offset offset, Offset length) {
+ Optional<Offset> read(Offset offset, char* buffer, Offset length) {
if (offset + length > size())
return {};
- std::string buffer;
- buffer.resize(length);
wrapped_->seek(offset);
- auto result = wrapped_->read(&buffer[0], length);
+ auto result = wrapped_->read(buffer, length);
if (result.is_error())
- // TODO: better error handling.
- return std::string{"[Bad Read]"};
+ return {};
- buffer.resize(*result);
- return buffer;
+ return *result;
}
- /* Returns the offset of the line in the newline cache if valid. */
- Optional<Offset> offset_for_line(Line line) const {
+ bool write(Offset offset, std::string_view value) {
+ wrapped_->seek(offset);
+ auto result = wrapped_->write(value.data(), value.length());
+
+ return result.is_ok();
+ }
+
+ /* Returns the index of the line in the newline cache if valid. */
+ Optional<Offset> index_for_line(Line line) const {
if (line >= line_count_)
return {};
@@ -193,8 +281,8 @@ class BufferWrapper {
if (line >= line_count_)
return;
- auto result = offset_for_line(line);
- if (result)
+ auto index = index_for_line(line);
+ if (index)
return;
if (line < start_line_) {
@@ -229,7 +317,7 @@ class BufferWrapper {
}
}
- /* Helpers for finding the prev/next newline. */
+ /* Finding the first newline backward from offset. */
Optional<Offset> previous_newline(Offset offset) {
char buffer[buffer_size];
auto to_read = buffer_size;
@@ -264,6 +352,7 @@ class BufferWrapper {
return {}; // Didn't find one.
}
+ /* Finding the first newline forward from offset. */
Optional<Offset> next_newline(Offset offset) {
// EOF, no more newlines to find.
if (offset >= size())
@@ -295,6 +384,65 @@ class BufferWrapper {
return size() - 1;
}
+ /* Grow the file and move file content so that the
+ * content at src is shifted forward by 'delta'. */
+ void expand(Offset src, int32_t delta) {
+ if (delta <= 0) // Not an expand.
+ return;
+
+ char buffer[buffer_size];
+ auto to_read = buffer_size;
+
+ // Number of bytes left to shift.
+ Offset remaining = size() - src;
+ Offset offset = size();
+
+ while (remaining > 0) {
+ offset -= std::min(remaining, buffer_size);
+ to_read = std::min(remaining, buffer_size);
+
+ wrapped_->seek(offset);
+ auto result = wrapped_->read(buffer, to_read);
+ if (result.is_error())
+ break;
+
+ wrapped_->seek(offset + delta);
+ result = wrapped_->write(buffer, *result);
+ if (result.is_error())
+ break;
+
+ remaining -= *result;
+ }
+ }
+
+ /* Shrink the file and move file content so that the
+ * content at src is shifted backward by 'delta'. */
+ void shrink(Offset src, int32_t delta) {
+ if (delta >= 0) // Not a shrink.
+ return;
+
+ char buffer[buffer_size];
+ auto offset = src;
+
+ while (true) {
+ wrapped_->seek(offset);
+ auto result = wrapped_->read(buffer, buffer_size);
+ if (result.is_error())
+ break;
+
+ wrapped_->seek(offset + delta);
+ result = wrapped_->write(buffer, *result);
+
+ if (result.is_error() || *result < buffer_size)
+ break;
+
+ offset += *result;
+ }
+
+ // Delete the extra bytes at the end of the file.
+ wrapped_->truncate();
+ }
+
BufferType* wrapped_{};
/* Total number of lines in the buffer. */
@@ -316,7 +464,7 @@ class FileWrapper : public BufferWrapper<File, 64> {
using Error = File::Error;
static Result<std::unique_ptr<FileWrapper>> open(const std::filesystem::path& path) {
auto fw = std::unique_ptr<FileWrapper>(new FileWrapper());
- auto error = fw->file_.open(path);
+ auto error = fw->file_.open(path, /*read_only*/ false);
if (error)
return *error;
@@ -325,6 +473,23 @@ class FileWrapper : public BufferWrapper<File, 64> {
return fw;
}
+ /* Underlying file. */
+ File& file() { return file_; }
+
+ /* Swaps out the underlying file for the specified file.
+ * The swapped file is expected have the same contents.
+ * For copy-on-write scenario with a temp file. */
+ bool assume_file(const std::filesystem::path& path) {
+ File file;
+ auto error = file.open(path, /*read_only*/ false);
+
+ if (error)
+ return false;
+
+ file_ = std::move(file);
+ return true;
+ }
+
private:
FileWrapper() {}
void initialize() {
diff --git a/firmware/application/ui_navigation.cpp b/firmware/application/ui_navigation.cpp
index 676222ba..cd2fc758 100644
--- a/firmware/application/ui_navigation.cpp
+++ b/firmware/application/ui_navigation.cpp
@@ -236,10 +236,8 @@ void SystemStatusView::refresh() {
if (portapack::clock_manager.get_reference().source == ClockManager::ReferenceSource::External) {
button_clock_status.set_bitmap(&bitmap_icon_clk_ext);
- // button_bias_tee.set_foreground(ui::Color::green()); Typo?
} else {
button_clock_status.set_bitmap(&bitmap_icon_clk_int);
- // button_bias_tee.set_foreground(ui::Color::green());
}
if (portapack::persistent_memory::clkout_enabled()) {
@@ -420,7 +418,7 @@ View* NavigationView::push_view(std::unique_ptr<View> new_view) {
free_view();
const auto p = new_view.get();
- view_stack.emplace_back(std::move(new_view));
+ view_stack.emplace_back(ViewState{std::move(new_view), {}});
update_view();
@@ -428,34 +426,14 @@ View* NavigationView::push_view(std::unique_ptr<View> new_view) {
}
void NavigationView::pop() {
- if (view() == modal_view) {
- modal_view = nullptr;
- }
-
- // Can't pop last item from stack.
- if (view_stack.size() > 1) {
- free_view();
-
- view_stack.pop_back();
-
- update_view();
- }
+ pop(true);
}
void NavigationView::pop_modal() {
- if (view() == modal_view) {
- modal_view = nullptr;
- }
-
- // Pop modal view + underlying app view
- if (view_stack.size() > 2) {
- free_view();
- view_stack.pop_back();
- free_view();
- view_stack.pop_back();
-
- update_view();
- }
+ // Pop modal view + underlying app view.
+ // TODO: this shouldn't be necessary.
+ pop(false);
+ pop(true);
}
void NavigationView::display_modal(
@@ -475,12 +453,31 @@ void NavigationView::display_modal(
}
}
+void NavigationView::pop(bool update) {
+ if (view() == modal_view) {
+ modal_view = nullptr;
+ }
+
+ // Can't pop last item from stack.
+ if (view_stack.size() > 1) {
+ auto on_pop = view_stack.back().on_pop;
+
+ free_view();
+ view_stack.pop_back();
+
+ if (update)
+ update_view();
+
+ if (on_pop) on_pop();
+ }
+}
+
void NavigationView::free_view() {
remove_child(view());
}
void NavigationView::update_view() {
- const auto new_view = view_stack.back().get();
+ const auto new_view = view_stack.back().view.get();
add_child(new_view);
new_view->set_parent_rect({{0, 0}, size()});
@@ -503,6 +500,18 @@ void NavigationView::focus() {
}
}
+bool NavigationView::set_on_pop(std::function<void()> on_pop) {
+ if (view_stack.size() <= 1)
+ return false;
+
+ auto& top = view_stack.back();
+ if (top.on_pop)
+ return false;
+
+ top.on_pop = on_pop;
+ return true;
+}
+
/* ReceiversMenuView *****************************************************/
ReceiversMenuView::ReceiversMenuView(NavigationView& nav) {
diff --git a/firmware/application/ui_navigation.hpp b/firmware/application/ui_navigation.hpp
index ba7313fb..4d3b8a25 100644
--- a/firmware/application/ui_navigation.hpp
+++ b/firmware/application/ui_navigation.hpp
@@ -75,9 +75,11 @@ class NavigationView : public View {
// Pushes a new view under the current on the stack so the current view returns into this new one.
template <class T, class... Args>
- void push_under_current(Args&&... args) {
+ T* push_under_current(Args&&... args) {
auto new_view = std::unique_ptr<View>(new T(*this, std::forward<Args>(args)...));
- view_stack.insert(view_stack.end() - 1, std::move(new_view));
+ auto new_view_ptr = new_view.get();
+ view_stack.insert(view_stack.end() - 1, ViewState{std::move(new_view), {}});
+ return reinterpret_cast<T*>(new_view_ptr);
}
template <class T, class... Args>
@@ -97,12 +99,22 @@ class NavigationView : public View {
void focus() override;
+ /* Sets the 'on_pop' handler for the current view.
+ * Returns true if the handler was bound successfully. */
+ bool set_on_pop(std::function<void()> on_pop);
+
private:
- std::vector<std::unique_ptr<View>> view_stack{};
+ struct ViewState {
+ std::unique_ptr<View> view;
+ std::function<void()> on_pop;
+ };
+
+ std::vector<ViewState> view_stack{};
Widget* modal_view{nullptr};
Widget* view() const;
+ void pop(bool update);
void free_view();
void update_view();
View* push_view(std::unique_ptr<View> new_view);
diff --git a/firmware/common/optional.hpp b/firmware/common/optional.hpp
index 7ff4c6d8..d382b957 100644
--- a/firmware/common/optional.hpp
+++ b/firmware/common/optional.hpp
@@ -43,10 +43,8 @@ class Optional {
const T& value() const& { return value_; }
const T& operator*() const& { return value_; }
- T&& value() && { return value_; }
- T&& operator*() && { return value_; }
- const T&& value() const&& { return value_; }
- const T&& operator*() const&& { return value_; }
+ T&& value() && { return std::move(value_); }
+ T&& operator*() && { return std::move(value_); }
T* operator->() { return &value_; }
const T* operator->() const { return &value_; }
diff --git a/firmware/common/ui_painter.cpp b/firmware/common/ui_painter.cpp
index fa3d7da6..b6edd8e8 100644
--- a/firmware/common/ui_painter.cpp
+++ b/firmware/common/ui_painter.cpp
@@ -35,18 +35,27 @@ Style Style::invert() const {
.foreground = background};
}
-int Painter::draw_char(const Point p, const Style& style, const char c) {
+int Painter::draw_char(Point p, const Style& style, char c) {
const auto glyph = style.font.glyph(c);
display.draw_glyph(p, glyph, style.foreground, style.background);
return glyph.advance().x();
}
-int Painter::draw_string(Point p, const Font& font, const Color foreground, const Color background, const std::string& text) {
+int Painter::draw_string(Point p, const Style& style, std::string_view text) {
+ return draw_string(p, style.font, style.foreground, style.background, text);
+}
+
+int Painter::draw_string(
+ Point p,
+ const Font& font,
+ Color foreground,
+ Color background,
+ std::string_view text) {
bool escape = false;
size_t width = 0;
Color pen = foreground;
- for (const auto c : text) {
+ for (auto c : text) {
if (escape) {
if (c <= 15)
pen = term_colors[c & 15];
@@ -65,48 +74,45 @@ int Painter::draw_string(Point p, const Font& font, const Color foreground, cons
}
}
}
+
return width;
}
-int Painter::draw_string(Point p, const Style& style, const std::string& text) {
- return draw_string(p, style.font, style.foreground, style.background, text);
-}
-
-void Painter::draw_bitmap(const Point p, const Bitmap& bitmap, const Color foreground, const Color background) {
+void Painter::draw_bitmap(Point p, const Bitmap& bitmap, Color foreground, Color background) {
display.draw_bitmap(p, bitmap.size, bitmap.data, foreground, background);
}
-void Painter::draw_hline(Point p, int width, const Color c) {
+void Painter::draw_hline(Point p, int width, Color c) {
display.fill_rectangle({p, {width, 1}}, c);
}
-void Painter::draw_vline(Point p, int height, const Color c) {
+void Painter::draw_vline(Point p, int height, Color c) {
display.fill_rectangle({p, {1, height}}, c);
}
-void Painter::draw_rectangle(const Rect r, const Color c) {
+void Painter::draw_rectangle(Rect r, Color c) {
draw_hline(r.location(), r.width(), c);
draw_vline({r.left(), r.top() + 1}, r.height() - 2, c);
draw_vline({r.left() + r.width() - 1, r.top() + 1}, r.height() - 2, c);
draw_hline({r.left(), r.top() + r.height() - 1}, r.width(), c);
}
-void Painter::fill_rectangle(const Rect r, const Color c) {
+void Painter::fill_rectangle(Rect r, Color c) {
display.fill_rectangle(r, c);
}
-void Painter::fill_rectangle_unrolled8(const Rect r, const Color c) {
+void Painter::fill_rectangle_unrolled8(Rect r, Color c) {
display.fill_rectangle_unrolled8(r, c);
}
-void Painter::paint_widget_tree(Widget* const w) {
+void Painter::paint_widget_tree(Widget* w) {
if (ui::is_dirty()) {
paint_widget(w);
ui::dirty_clear();
}
}
-void Painter::paint_widget(Widget* const w) {
+void Painter::paint_widget(Widget* w) {
if (w->hidden()) {
// Mark widget (and all children) as invisible.
w->visible(false);
diff --git a/firmware/common/ui_painter.hpp b/firmware/common/ui_painter.hpp
index 24f7835e..c71c2303 100644
--- a/firmware/common/ui_painter.hpp
+++ b/firmware/common/ui_painter.hpp
@@ -25,7 +25,7 @@
#include "ui.hpp"
#include "ui_text.hpp"
-#include <string>
+#include <string_view>
namespace ui {
@@ -46,24 +46,24 @@ class Painter {
Painter(const Painter&) = delete;
Painter(Painter&&) = delete;
- int draw_char(const Point p, const Style& style, const char c);
+ int draw_char(Point p, const Style& style, char c);
- int draw_string(Point p, const Font& font, const Color foreground, const Color background, const std::string& text);
- int draw_string(Point p, const Style& style, const std::string& text);
+ int draw_string(Point p, const Style& style, std::string_view text);
+ int draw_string(Point p, const Font& font, Color foreground, Color background, std::string_view text);
- void draw_bitmap(const Point p, const Bitmap& bitmap, const Color background, const Color foreground);
+ void draw_bitmap(Point p, const Bitmap& bitmap, Color background, Color foreground);
- void draw_rectangle(const Rect r, const Color c);
- void fill_rectangle(const Rect r, const Color c);
- void fill_rectangle_unrolled8(const Rect r, const Color c);
+ void draw_rectangle(Rect r, Color c);
+ void fill_rectangle(Rect r, Color c);
+ void fill_rectangle_unrolled8(Rect r, Color c);
- void paint_widget_tree(Widget* const w);
+ void paint_widget_tree(Widget* w);
- void draw_hline(Point p, int width, const Color c);
- void draw_vline(Point p, int height, const Color c);
+ void draw_hline(Point p, int width, Color c);
+ void draw_vline(Point p, int height, Color c);
private:
- void paint_widget(Widget* const w);
+ void paint_widget(Widget* w);
};
} /* namespace ui */
diff --git a/firmware/test/application/test_file_wrapper.cpp b/firmware/test/application/test_file_wrapper.cpp
index dd0b22b6..98376097 100644
--- a/firmware/test/application/test_file_wrapper.cpp
+++ b/firmware/test/application/test_file_wrapper.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2023
+ * Copyright (C) 2023 Kyle Reed
*
* This file is part of PortaPack.
*
@@ -42,14 +42,23 @@ class MockFile {
Size size() { return data_.size(); }
Result<Offset> seek(uint32_t offset) {
- if (offset >= size())
+ if ((int32_t)offset < 0)
return {static_cast<Error>(FR_BAD_SEEK)};
auto previous = offset_;
+
+ if (offset > size())
+ data_.resize(offset);
+
offset_ = offset;
return previous;
}
+ Result<Offset> truncate() {
+ data_.resize(offset_);
+ return offset_;
+ }
+
Result<Size> read(void* data, Size bytes_to_read) {
if (offset_ + bytes_to_read > size())
bytes_to_read = size() - offset_;
@@ -62,6 +71,20 @@ class MockFile {
return bytes_to_read;
}
+ Result<Size> write(const void* data, Size bytes_to_write) {
+ auto new_offset = offset_ + bytes_to_write;
+ if (new_offset >= size())
+ data_.resize(new_offset);
+
+ memcpy(&data_[offset_], data, bytes_to_write);
+ offset_ = new_offset;
+ return bytes_to_write;
+ }
+
+ Optional<Error> sync() {
+ return {};
+ }
+
std::string data_;
uint32_t offset_{0};
};
@@ -89,11 +112,19 @@ TEST_SUITE("Test MockFile") {
SCENARIO("File seek") {
GIVEN("Valid file") {
MockFile f{"abc\ndef"};
+ auto init_size = f.size();
+
+ WHEN("seek()") {
+ f.seek(4);
+ THEN("offset_ should be updated.") {
+ CHECK_EQ(f.offset_, 4);
+ }
+ }
WHEN("seek() negative offset") {
auto r = f.seek(-1);
- THEN("Result should be bad_seek") {
+ THEN("Result should be bad_seek.") {
CHECK(r.is_error());
CHECK_EQ(r.error().code(), FR_BAD_SEEK);
}
@@ -102,25 +133,25 @@ TEST_SUITE("Test MockFile") {
WHEN("seek() offset is size()") {
auto r = f.seek(f.size());
- THEN("Result should be bad_seek") {
- CHECK(r.is_error());
- CHECK_EQ(r.error().code(), FR_BAD_SEEK);
+ THEN("File should not grow.") {
+ CHECK(r.is_ok());
+ CHECK_EQ(f.size(), init_size);
}
}
WHEN("seek() offset > size()") {
auto r = f.seek(f.size() + 1);
- THEN("Result should be bad_seek") {
- CHECK(r.is_error());
- CHECK_EQ(r.error().code(), FR_BAD_SEEK);
+ THEN("File should grow.") {
+ CHECK(r.is_ok());
+ CHECK_EQ(f.size(), init_size + 1);
}
}
WHEN("seek() offset < size()") {
auto r = f.seek(1);
- THEN("Result should be ok") {
+ THEN("Result should be ok.") {
CHECK(r.is_ok());
}
@@ -185,6 +216,72 @@ TEST_SUITE("Test MockFile") {
}
}
}
+
+ SCENARIO("File write") {
+ GIVEN("Valid file") {
+ MockFile f{"abc\ndef"};
+
+ WHEN("Writing over existing region") {
+ f.write("xyz", 3);
+
+ THEN("It should overwrite") {
+ CHECK_EQ(f.data_, "xyz\ndef");
+ }
+ }
+
+ WHEN("Writing over past end") {
+ f.seek(f.size());
+ f.write("xyz", 3);
+
+ THEN("It should extend file and write") {
+ CHECK_EQ(f.size(), 10);
+ CHECK_EQ(f.data_, "abc\ndefxyz");
+ }
+ }
+ }
+ }
+
+ // This scenario was tested on device.
+ SCENARIO("File truncate") {
+ GIVEN("Valid file") {
+ MockFile f{"hello world"};
+
+ WHEN("truncating at offset 5") {
+ f.seek(5);
+ f.truncate();
+
+ THEN("resulting file should be 'hello'.") {
+ CHECK_EQ(f.size(), 5);
+ CHECK_EQ(f.data_, "hello");
+ }
+ }
+ }
+ }
+
+ SCENARIO("File truncate") {
+ GIVEN("Valid file") {
+ MockFile f{"abc\ndef"};
+ auto init_size = f.size();
+
+ WHEN("R/W pointer at end") {
+ f.seek(f.size());
+ f.truncate();
+
+ THEN("It should not change size.") {
+ CHECK_EQ(f.size(), init_size);
+ }
+ }
+
+ WHEN("R/W pointer in middle") {
+ f.seek(3);
+ f.truncate();
+
+ THEN("It should change size.") {
+ CHECK_EQ(f.size(), 3);
+ }
+ }
+ }
+ }
}
TEST_SUITE_BEGIN("Test BufferWrapper");
@@ -343,4 +440,259 @@ SCENARIO("Reading with cache miss.") {
}
}
+SCENARIO("Replace range of same size.") {
+ GIVEN("A file with lines") {
+ MockFile f{"abc\ndef"};
+ auto w = wrap_buffer(f);
+ auto init_line_count = w.line_count();
+ auto init_size = w.size();
+
+ WHEN("Replacing range without changing size") {
+ w.replace_range({0, 3}, "xyz");
+
+ CHECK_EQ("xyz\ndef", f.data_);
+
+ THEN("size should not change.") {
+ CHECK_EQ(w.size(), init_size);
+ }
+
+ THEN("text should be replaced.") {
+ auto str = w.get_text(0, 0, 10);
+ REQUIRE(str);
+ CHECK_EQ(*str, "xyz\n");
+ }
+ }
+ }
+}
+
+SCENARIO("Replace range that increases size.") {
+ GIVEN("A file with lines") {
+ MockFile f{"abc\ndef"};
+ auto w = wrap_buffer(f);
+ auto init_line_count = w.line_count();
+ auto init_size = w.size();
+
+ WHEN("Replacing range with larger size") {
+ w.replace_range({0, 3}, "wxyz");
+
+ CHECK_EQ(f.data_, "wxyz\ndef");
+
+ THEN("size should be increased.") {
+ CHECK_EQ(w.size(), init_size + 1);
+ }
+
+ THEN("text should be replaced.") {
+ auto str = w.get_text(0, 0, 10);
+ REQUIRE(str);
+ CHECK_EQ(*str, "wxyz\n");
+ }
+
+ THEN("following text should not be modified.") {
+ auto str = w.get_text(1, 0, 10);
+ REQUIRE(str);
+ CHECK_EQ(*str, "def");
+ }
+ }
+ }
+
+ GIVEN("A file larger than internal buffer_size (512)") {
+ std::string content = std::string(599, 'a');
+ content.push_back('x');
+ MockFile f{content};
+
+ auto w = wrap_buffer(f);
+ auto init_line_count = w.line_count();
+ auto init_size = w.size();
+
+ WHEN("Replacing range with larger size") {
+ w.replace_range({0, 2}, "bbb");
+
+ THEN("size should be increased.") {
+ CHECK_EQ(w.size(), init_size + 1);
+ }
+
+ THEN("text should be replaced.") {
+ auto str = w.get_text(0, 0, 10);
+ REQUIRE(str);
+ CHECK_EQ(*str, "bbbaaaaaaa");
+ }
+
+ THEN("end text should be preserved.") {
+ CHECK_EQ(f.data_.back(), 'x');
+ }
+ }
+ }
+}
+
+SCENARIO("Replace range that decreases size.") {
+ GIVEN("A file with lines") {
+ MockFile f{"abc\ndef"};
+ auto w = wrap_buffer(f);
+ auto init_line_count = w.line_count();
+ auto init_size = w.size();
+
+ WHEN("Replacing range with smaller size") {
+ w.replace_range({0, 3}, "yz");
+
+ CHECK_EQ(f.data_, "yz\ndef");
+
+ THEN("size should be decreased.") {
+ CHECK_EQ(w.size(), init_size - 1);
+ }
+
+ THEN("text should be replaced.") {
+ auto str = w.get_text(0, 0, 10);
+ REQUIRE(str);
+ CHECK_EQ(*str, "yz\n");
+ }
+
+ THEN("following text should not be modified.") {
+ auto str = w.get_text(1, 0, 10);
+ REQUIRE(str);
+ CHECK_EQ(*str, "def");
+ }
+ }
+ }
+
+ GIVEN("A file larger than internal buffer_size (512)") {
+ std::string content = std::string(599, 'a');
+ content.push_back('x');
+ MockFile f{content};
+
+ auto w = wrap_buffer(f);
+ auto init_line_count = w.line_count();
+ auto init_size = w.size();
+
+ WHEN("Replacing range with smaller size") {
+ w.replace_range({0, 10}, "b");
+
+ THEN("size should be decreased.") {
+ CHECK_EQ(w.size(), init_size - 9);
+ }
+
+ THEN("text should be replaced.") {
+ auto str = w.get_text(0, 0, 10);
+ REQUIRE(str);
+ CHECK_EQ(*str, "baaaaaaaaa");
+ }
+
+ THEN("end should be moved toward front.") {
+ CHECK_EQ(f.data_.back(), 'x');
+ }
+ }
+ }
+}
+
+SCENARIO("Insert line.") {
+ GIVEN("A file with lines") {
+ MockFile f{"abc\ndef\nghi\n"};
+ auto w = wrap_buffer(f);
+ auto init_line_count = w.line_count();
+ auto init_size = w.size();
+
+ WHEN("Inserting before the first line") {
+ w.insert_line(0);
+
+ THEN("should increment line count and size.") {
+ CHECK_EQ(w.line_count(), init_line_count + 1);
+ CHECK_EQ(w.size(), init_size + 1);
+ }
+
+ THEN("should insert empty line content.") {
+ auto str = w.get_text(0, 0, 10);
+ CHECK_EQ(*str, "\n");
+ }
+
+ THEN("first line should be pushed down.") {
+ auto str = w.get_text(1, 0, 10);
+ CHECK_EQ(*str, "abc\n");
+ }
+ }
+
+ WHEN("Inserting before the last line") {
+ w.insert_line(2);
+
+ THEN("should increment line count and size.") {
+ CHECK_EQ(w.line_count(), init_line_count + 1);
+ CHECK_EQ(w.size(), init_size + 1);
+ }
+
+ THEN("should insert empty line content.") {
+ auto str = w.get_text(2, 0, 10);
+ CHECK_EQ(*str, "\n");
+ }
+
+ THEN("last line should be pushed down.") {
+ auto str = w.get_text(3, 0, 10);
+ CHECK_EQ(*str, "ghi\n");
+ }
+ }
+
+ WHEN("Inserting after the last line") {
+ w.insert_line(w.line_count());
+
+ THEN("should increment line count and size.") {
+ CHECK_EQ(w.line_count(), init_line_count + 1);
+ CHECK_EQ(w.size(), init_size + 1);
+ }
+
+ THEN("should insert empty line content.") {
+ auto str = w.get_text(3, 0, 10);
+ CHECK_EQ(*str, "\n");
+ }
+ }
+ }
+}
+
+SCENARIO("Delete line.") {
+ GIVEN("A file with lines") {
+ MockFile f{"abc\ndef\nghi"};
+ auto w = wrap_buffer(f);
+ auto init_line_count = w.line_count();
+ auto init_size = w.size();
+
+ WHEN("Deleting the first line") {
+ w.delete_line(0);
+
+ THEN("should decrement line count and size.") {
+ CHECK_EQ(w.line_count(), init_line_count - 1);
+ CHECK_EQ(w.size(), init_size - 4);
+ }
+
+ THEN("should remove first line content.") {
+ auto str = w.get_text(0, 0, 10);
+ CHECK_EQ(*str, "def\n");
+ }
+ }
+
+ WHEN("Deleting the middle line") {
+ w.delete_line(1);
+
+ THEN("should decrement line count and size.") {
+ CHECK_EQ(w.line_count(), init_line_count - 1);
+ CHECK_EQ(w.size(), init_size - 4);
+ }
+
+ THEN("should remove middle line content.") {
+ auto str = w.get_text(1, 0, 10);
+ CHECK_EQ(*str, "ghi");
+ }
+ }
+
+ WHEN("Deleting the last line") {
+ w.delete_line(2);
+
+ THEN("should decrement line count and size.") {
+ CHECK_EQ(w.line_count(), init_line_count - 1);
+ CHECK_EQ(w.size(), init_size - 3);
+ }
+
+ THEN("should remove last line content.") {
+ auto str = w.get_text(2, 0, 10);
+ CHECK(!str);
+ }
+ }
+ }
+}
+
TEST_SUITE_END();
\ No newline at end of file