Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ The release run heads these entries with the version and opens a fresh
`"version": 1` while `Document::edit` takes 2, so every save the browser
produced was refused. A check page now asserts the version.

- A `.txt` view carries `odr.editing` like every other view, rather than the
second API `text.js` was. **Breaking**: `HtmlConfig::editable` no longer
writes `contenteditable` — `odr.editing.enable()` does.

- `TextFile::write_edited` saves an edited plain file and `is_savable()` says
whether it can; `txt` states `edit` and `save`. The output is UTF-8 whatever
the source encoding was, because there is no encoder back.

- A `.pptx` can be edited and saved: every text operation a `.docx` takes, and
a save that writes the slide parts back into the package. Its
`FileTypeCapabilities` now states `edit` and `save`.
Expand Down
11 changes: 6 additions & 5 deletions docs/design/document-editing.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
The editor of the **document view** — `frontend/document.js` — built on the
mode frame in [`editing.md`](editing.md), and the decisions that are its own.
[`spreadsheet-editing.md`](spreadsheet-editing.md) is the sibling document for
the sheet view. Text documents, presentations and drawings share this one:
what it edits is runs and paragraphs, wherever the format puts them.
the sheet view and [`txt-editing.md`](txt-editing.md) for the plain-text one.
Text documents, presentations and drawings share this one: what it edits is
runs and paragraphs, wherever the format puts them.

Status: **landed.** The schema, the replay, the browser editor and the pptx
write side are all in the code; each section says what is in and what is not.
Expand Down Expand Up @@ -274,6 +275,7 @@ reading forward.
| `.odt`, `.odp`, `.ods`, `.odg` | `odf` | edits and saves today; the new ops land here |
| `.docx` | `ooxml/text` | edits and saves today; the new ops land here |
| `.pptx` | `ooxml/presentation` | edits and saves; the same operations over `a:p` / `a:r` |
| `.txt` | `text` | not a document at all; see [`txt-editing.md`](txt-editing.md) |
| everything else | — | read-only, and says so by decision 7 |

`.odp` needs nothing of its own: a presentation is the same odf `Document` as a
Expand Down Expand Up @@ -350,6 +352,5 @@ Two details the checks pin down:
- **A list item** is a paragraph in a list. Enter at the end of one should make
a new list item, not a bare paragraph. `splitParagraph` splits what the
element tree says is a paragraph; the list case is not covered.
- The **plain-text view** (`html/text_file.cpp`) is still its own editor and
still answers to nobody. Unchanged by this work, and still the open question
at the end of [`editing.md`](editing.md).
- The **plain-text view** is a `TextFile` rather than a document, so none of
this reaches it; [`txt-editing.md`](txt-editing.md) is its own.
23 changes: 12 additions & 11 deletions docs/design/editing.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ editing of ODF and OOXML documents, the alternatives we weighed, and *why* we
took each decision. Decisions 9 to 12 are the frame every format shares, and
they are in the code.

[`spreadsheet-editing.md`](spreadsheet-editing.md) is the first editor built on
the frame, and it is where a sheet's own decisions live.
[`document-editing.md`](document-editing.md) is the second, and it is where the phases
below are being carried out — an edit across runs, a new paragraph, and the
delete and replace that reach across both.
One document per editor, each holding its own decisions:
[`spreadsheet-editing.md`](spreadsheet-editing.md) for the sheet view,
[`document-editing.md`](document-editing.md) for the document view — where the
phases below are carried out — and [`txt-editing.md`](txt-editing.md) for the
plain-text one.

This builds on the existing principle in [`README.md`](README.md):

Expand Down Expand Up @@ -561,9 +561,10 @@ session-scoped, decision 4).
[`spreadsheet-editing.md`](spreadsheet-editing.md)). Does the adapter hook
grow into `element_edit_lock(id) -> reason`, or does the renderer keep
deciding the reason from the element it is over?
- The plain-text view (`html/text_file.cpp`) is outside the mode: `text.js` is
its own editor, with its own `beforeinput` interception and its own undo, and
`config.editable` writes the `contenteditable` it needs. Nothing replays those
edits into a file, because `txt` declares no `edit` capability. Does that view
attach to `odr.editing` — which would need an editable-but-not-savable state —
or stay the one editor that answers to nobody?
- ~~The plain-text view is outside the mode.~~ **Answered: it attaches.**
See [`txt-editing.md`](txt-editing.md).
- The **pdf annotator** is now the one editor that answers to nobody:
`odr.annotation` is its own API and `PdfFile::annotate` its own write path.
It is a different gesture from editing text, so whether it should share the
mode is a real question rather than an oversight
([`txt-editing.md`](txt-editing.md) carries it too).
117 changes: 117 additions & 0 deletions docs/design/txt-editing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# Plain-text editing design

The editor of the **plain-text view** — `frontend/text.js` — built on the mode
frame in [`editing.md`](editing.md), and the decisions that are its own. Its
siblings are [`document-editing.md`](document-editing.md) for a document view
and [`spreadsheet-editing.md`](spreadsheet-editing.md) for a sheet.

Status: **landed.** It answers the question `editing.md` carried from the day
the mode was built — whether this view attaches to `odr.editing` or stays the
one editor that answers to nobody. It attaches.

## What makes it different

A `.txt` is a `TextFile`, **not a `DocumentFile`**. There is no element tree
behind it: no runs, no paragraphs, no ids, no registry, and no adapters. So
none of the document operations reach it, `Document::edit` and `Document::save`
do not apply to it, and the whole write side of the engines is somewhere it
cannot go.

What it has instead is a flat string of lines and, in the browser, an editor
that predates all of this — `text.js` had its own `beforeinput` gate and its
own inverse-recording undo before the mode existed.

## Decisions

### 1. It attaches to the mode rather than keeping its own API

`text.js` is an editor on `odr.editing` like every format's. The view writes
`editing.js` and the page-level `data-odr-editable` / `data-odr-keyboard`.

**Why:** decision 9 of [`editing.md`](editing.md) is that a host wires the mode
once for every file it opens and must not learn a second API because of what
the file turned out to be. Decision 11 states that `odr.editing` is on every
view either way and `odr.generateDiff()` never goes missing. Both were true of
every view **except this one**: `text_file.cpp` wrote `text.js` and not
`editing.js`, so a `.txt` page had no mode at all and `odr.generateDiff` came
back `undefined`.

**What it cost:** nothing in the editor. `text.js` already recorded an inverse
per change, so `undo`, `redo`, `canUndo` and `canRedo` answered honestly the
moment they were wired up. It is the only editor of the three that did not need
an undo built for it.

### 2. One operation, `setContent {text}`, carrying the whole file

```json
{"version": 2, "ops": [{"op": "setContent", "text": "…"}]}
```

Coalescing makes the log exactly one operation however long the session runs,
and an envelope stating none writes the file back as it was.

**Why not a line at a time**, which is the obvious alternative: **a line number
is a path**, and decision 1 of [`document-editing.md`](document-editing.md) is
that an operation must not address by one — inserting a line shifts every line
after it, so a log of more than one structural operation cannot be replayed. A
document escapes that with ids from its registry. A plain file has no registry
to hang an id on, because lines are not elements; they are where the newlines
happen to be. Per-line operations would mean inventing an identity the format
does not have.

**Why not finer:** addressing inside a line wants offsets, and decision 2 of
[`document-editing.md`](document-editing.md) refused those — JavaScript counts
UTF-16 code units and `std::string` counts bytes.

**What it costs:** the whole file crosses the bridge on every save. Less than
it looks, because `write_edited` produces the complete bytes either way, so a
finer log would only be reassembled before writing; the saving would be one
hop. Where it does bite is a large file, and the answer there is **one**
`replaceLines {from, to, text}` computed as a single diff hunk at emit time —
still one operation, still applied to the file as it was, so still nothing
positional to go stale. That needs no schema change to reach.

### 3. The write path is `PdfFile::annotate`'s shape, not `Document::save`'s

```cpp
[[nodiscard]] bool TextFile::is_savable() const noexcept;
void TextFile::write_edited(std::string_view operations, std::ostream &out,
const Logger & = Logger::null()) const;
```

One call taking the envelope and a stream, leaving the handle unchanged.

**Why:** a `TextFile` is an immutable handle over bytes, and there is no
document to mutate and later serialise. `PdfFile::annotate` is the precedent —
the other non-document file with a write path of its own — and the shape suits
for the same reason: nothing is held between the edit and the write.

### 4. What it writes is UTF-8, whatever the source was

`is_savable()` refuses only an encoding we cannot **decode**: the view hands
those bytes to the browser as they are, so what comes back could not be put
back. Everything else saves — and saves as UTF-8.

**Why:** `encoding/transcode.hpp` has `to_utf8` and nothing in the other
direction. A Shift-JIS file therefore opens, edits, saves, and is UTF-8
afterwards.

**Why it is stated rather than hidden:** the API doc says so, so a host can
warn the reader. Silently changing a property of someone's file that nothing
told them about is the failure mode worth avoiding here, more than the change
itself.

## Open questions

- **`from_utf8`** would let a file round-trip in its own encoding and close
decision 4. The tables in `encoding/encoding_data` are there to reverse, so
the work is real but bounded — except for the question it brings with it:
what to do with a character the target encoding has no room for.
- **Undo granularity.** Typing over a selection is two steps, because
`insertTextAction` calls `removeTextAction` and each pushes its own change.
The document editor is one `beforeinput`, one step. It predates this work and
is only visible now that a host can drive undo.
- **The pdf annotator** is now the one editor that answers to nobody:
`odr.annotation` is its own API and `PdfFile::annotate` its own write path.
That is a different gesture from editing text, so whether it should share the
mode is a real question rather than an oversight.
36 changes: 36 additions & 0 deletions src/odr/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
#include <odr/internal/util/file_util.hpp>
#include <odr/internal/util/stream_util.hpp>

#include <nlohmann/json.hpp>

#include <optional>
#include <ostream>
#include <stdexcept>

namespace odr {

Expand Down Expand Up @@ -286,6 +290,38 @@ std::string TextFile::text() const {
return internal::encoding::to_utf8(bytes, encoding);
}

bool TextFile::is_savable() const noexcept {
const TextEncoding encoding = this->encoding();
return encoding == TextEncoding::unknown ||
text_encoding_is_decodable(encoding);
}

void TextFile::write_edited(const std::string_view operations,
std::ostream &out,
const Logger & /*logger*/) const {
if (!is_savable()) {
throw UnsupportedOperation();
}

const nlohmann::json json = nlohmann::json::parse(operations);
if (json.value("version", 0) != 2) {
throw std::invalid_argument("unsupported edit version");
}

std::optional<std::string> content;
for (const nlohmann::json &operation : json.at("ops")) {
const auto name = operation.at("op").get<std::string>();
if (name != "setContent") {
throw std::invalid_argument("unknown operation " + name);
}
content = operation.at("text").get<std::string>();
}

// an envelope stating any operation replaces every byte, so the file is
// only read where it states none
out << (content.has_value() ? *content : text());
}

std::shared_ptr<internal::abstract::TextFile> TextFile::impl() const {
return m_impl;
}
Expand Down
15 changes: 14 additions & 1 deletion src/odr/file.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <odr/logger.hpp>

#include <iosfwd>
#include <memory>
#include <optional>
#include <string>
Expand Down Expand Up @@ -202,7 +203,8 @@ struct FileTypeCapabilities final {
///< paints
bool color_scheme{}; ///< the view honors @ref HtmlConfig::color_scheme
bool edit{}; ///< @ref Document::is_editable can be `true`
bool save{}; ///< @ref Document::save is supported
bool save{}; ///< @ref Document::save, or for a plain file
///< @ref TextFile::write_edited
bool encrypt{}; ///< @ref Document::save with a password is supported
bool annotate{}; ///< @ref PdfFile::annotate is supported; a concrete file
///< still answers for itself with
Expand Down Expand Up @@ -436,6 +438,17 @@ class TextFile final : public DecodedFile {
/// the raw bytes where it is not.
[[nodiscard]] std::string text() const;

/// False where @ref encoding cannot be decoded: the view hands those bytes
/// to the browser as they are, so what comes back cannot be put back.
[[nodiscard]] bool is_savable() const noexcept;

/// Applies @p operations - `{"version": 2, "ops": [{"op": "setContent",
/// "text": "…"}]}` - and writes the result to @p out, as UTF-8 whatever
/// @ref encoding the source was. See `docs/design/txt-editing.md`.
/// @throws UnsupportedOperation where @ref is_savable is false.
void write_edited(std::string_view operations, std::ostream &out,
const Logger &logger = Logger::null()) const;

[[nodiscard]] std::shared_ptr<internal::abstract::TextFile> impl() const;

private:
Expand Down
4 changes: 3 additions & 1 deletion src/odr/internal/file_type_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,9 @@ constexpr std::array table{
{.detect_by_content = true,
.open = true,
.translate_html = true,
.color_scheme = true}},
.color_scheme = true,
.edit = true,
.save = true}},
Row{FileType::comma_separated_values,
"csv"sv,
csv_extensions,
Expand Down
14 changes: 14 additions & 0 deletions src/odr/internal/html/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,4 +345,18 @@ std::string html::file_to_url(const abstract::File &file,
return file_to_url(*file.stream(), mime_type);
}

std::string html::keyboard_classes(const HtmlConfig &config) {
std::string classes;
if (config.keyboard_navigation) {
classes += "navigation";
}
if (config.keyboard_shortcuts) {
if (!classes.empty()) {
classes += " ";
}
classes += "shortcuts";
}
return classes;
}

} // namespace odr::internal
3 changes: 3 additions & 0 deletions src/odr/internal/html/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,7 @@ std::string file_to_url(std::istream &file, const std::string &mime_type);
std::string file_to_url(const abstract::File &file,
const std::string &mime_type);

/// The key classes a view's scripts may take; `editing.md` decision 12.
[[nodiscard]] std::string keyboard_classes(const HtmlConfig &config);

} // namespace odr::internal::html
15 changes: 0 additions & 15 deletions src/odr/internal/html/document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,21 +198,6 @@ void write_head(const Document &document, const WritingState &state,
out.write_header_end();
}

/// The key classes the view's scripts may take; `editing.md` decision 12.
std::string keyboard_classes(const HtmlConfig &config) {
std::string classes;
if (config.keyboard_navigation) {
classes += "navigation";
}
if (config.keyboard_shortcuts) {
if (!classes.empty()) {
classes += " ";
}
classes += "shortcuts";
}
return classes;
}

void write_body_begin(const Document &document, const WritingState &state) {
HtmlWriter &out = state.out();

Expand Down
6 changes: 3 additions & 3 deletions src/odr/internal/html/frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ void html::write_viewport_script(const WritingState &state) {
}

HtmlResources html::locate_text_resources(const HtmlConfig &config) {
static constexpr std::array assets{text_css_asset, search_css_asset,
search_js_asset, text_js_asset,
viewport_js_asset};
static constexpr std::array assets{text_css_asset, search_css_asset,
search_js_asset, editing_js_asset,
text_js_asset, viewport_js_asset};
static constexpr std::array dark{text_dark_css_asset, search_dark_css_asset};
return locate_all(assets, dark, config);
}
Expand Down
Loading
Loading