diff --git a/pyWebLayout/core/highlight.py b/pyWebLayout/core/highlight.py
index 20a428b..cdc1597 100644
--- a/pyWebLayout/core/highlight.py
+++ b/pyWebLayout/core/highlight.py
@@ -6,12 +6,16 @@ managing highlight collections, and rendering highlights on pages.
"""
from __future__ import annotations
+import logging
from dataclasses import dataclass
from typing import List, Tuple, Optional, Dict, Any
from enum import Enum
-import json
from pathlib import Path
+from pyWebLayout.core.persistence import ensure_dir, read_json, write_json
+
+logger = logging.getLogger(__name__)
+
class HighlightColor(Enum):
"""Predefined highlight colors with RGBA values"""
@@ -44,6 +48,12 @@ class Highlight:
start_word_index: Optional[int] = None # Word index in document (if available)
end_word_index: Optional[int] = None
+ # Where in the document this highlight lives, as a serialized
+ # RenderingPosition. `bounds` are pixel coordinates on one particular
+ # rendering, so they stop matching as soon as the font scale or page size
+ # changes; this survives repagination and is what page association uses.
+ position: Optional[Dict[str, Any]] = None
+
# Metadata
note: Optional[str] = None # Optional annotation
tags: List[str] = None # Optional categorization tags
@@ -63,6 +73,7 @@ class Highlight:
'text': self.text,
'start_word_index': self.start_word_index,
'end_word_index': self.end_word_index,
+ 'position': self.position,
'note': self.note,
'tags': self.tags,
'timestamp': self.timestamp
@@ -78,6 +89,7 @@ class Highlight:
text=data['text'],
start_word_index=data.get('start_word_index'),
end_word_index=data.get('end_word_index'),
+ position=data.get('position'),
note=data.get('note'),
tags=data.get('tags', []),
timestamp=data.get('timestamp')
@@ -100,12 +112,9 @@ class HighlightManager:
highlights_dir: Directory to store highlight data
"""
self.document_id = document_id
- self.highlights_dir = Path(highlights_dir)
+ self.highlights_dir = ensure_dir(highlights_dir)
self.highlights: Dict[str, Highlight] = {} # id -> Highlight
- # Create directory if it doesn't exist
- self.highlights_dir.mkdir(parents=True, exist_ok=True)
-
# Load existing highlights
self._load_highlights()
@@ -178,34 +187,22 @@ class HighlightManager:
def _save_highlights(self) -> None:
"""Persist highlights to disk"""
- try:
- filepath = self._get_filepath()
- data = {
- 'document_id': self.document_id,
- 'highlights': [h.to_dict() for h in self.highlights.values()]
- }
-
- with open(filepath, 'w') as f:
- json.dump(data, f, indent=2)
- except Exception as e:
- print(f"Error saving highlights: {e}")
+ write_json(self._get_filepath(), {
+ 'document_id': self.document_id,
+ 'highlights': [h.to_dict() for h in self.highlights.values()]
+ })
def _load_highlights(self) -> None:
"""Load highlights from disk"""
+ data = read_json(self._get_filepath(), {})
try:
- filepath = self._get_filepath()
- if not filepath.exists():
- return
-
- with open(filepath, 'r') as f:
- data = json.load(f)
-
self.highlights = {
h['id']: Highlight.from_dict(h)
for h in data.get('highlights', [])
}
- except Exception as e:
- print(f"Error loading highlights: {e}")
+ except (AttributeError, TypeError, KeyError):
+ logger.warning("Highlight file %s is not in the expected shape; ignoring it",
+ self._get_filepath(), exc_info=True)
self.highlights = {}
@@ -213,16 +210,18 @@ def create_highlight_from_query_result(
result,
color: Tuple[int, int, int, int] = HighlightColor.YELLOW.value,
note: Optional[str] = None,
- tags: Optional[List[str]] = None
+ tags: Optional[List[str]] = None,
+ position: Optional[Dict[str, Any]] = None
) -> Highlight:
"""
Create a highlight from a QueryResult.
Args:
- result: QueryResult from query_pixel or query_range
+ result: QueryResult from query_point or query_range
color: RGBA color tuple
note: Optional annotation
tags: Optional categorization tags
+ position: Serialized RenderingPosition of the page the result came from
Returns:
Highlight instance
@@ -243,6 +242,7 @@ def create_highlight_from_query_result(
bounds=bounds,
color=color,
text=text,
+ position=position,
note=note,
tags=tags or [],
timestamp=time()
diff --git a/pyWebLayout/core/persistence.py b/pyWebLayout/core/persistence.py
new file mode 100644
index 0000000..89b585a
--- /dev/null
+++ b/pyWebLayout/core/persistence.py
@@ -0,0 +1,59 @@
+"""
+Small JSON-file helpers shared by the per-document stores.
+
+BookmarkManager and HighlightManager both keep a JSON file per document under a
+directory, and both had their own copy of "make the directory, try to read it,
+swallow and print on failure". The duplication is the point of this module; the
+file formats themselves stay owned by each store.
+"""
+
+from __future__ import annotations
+
+import json
+import logging
+from pathlib import Path
+from typing import Any
+
+logger = logging.getLogger(__name__)
+
+
+def ensure_dir(path: str | Path) -> Path:
+ """Return `path` as a Path, creating it and any missing parents."""
+ directory = Path(path)
+ directory.mkdir(parents=True, exist_ok=True)
+ return directory
+
+
+def read_json(path: Path, default: Any) -> Any:
+ """
+ Read JSON from `path`, returning `default` if it is missing or unreadable.
+
+ A corrupt store must not stop a book from opening, so failures are logged
+ and swallowed. `default` is returned as given, so pass a fresh mutable if
+ the caller intends to mutate it.
+ """
+ if not path.exists():
+ return default
+
+ try:
+ with open(path, 'r', encoding='utf-8') as handle:
+ return json.load(handle)
+ except (OSError, ValueError):
+ logger.warning("Could not read %s; ignoring its contents", path, exc_info=True)
+ return default
+
+
+def write_json(path: Path, data: Any) -> bool:
+ """
+ Write `data` to `path` as JSON.
+
+ Returns True on success. Failures are logged rather than raised: losing a
+ bookmark is not a reason to take down the reader.
+ """
+ try:
+ with open(path, 'w', encoding='utf-8') as handle:
+ json.dump(data, handle, indent=2)
+ return True
+ except (OSError, TypeError, ValueError):
+ logger.error("Could not write %s", path, exc_info=True)
+ return False
diff --git a/pyWebLayout/layout/ereader_manager.py b/pyWebLayout/layout/ereader_manager.py
index 736daf4..c6f63ea 100644
--- a/pyWebLayout/layout/ereader_manager.py
+++ b/pyWebLayout/layout/ereader_manager.py
@@ -8,9 +8,7 @@ into a unified, easy-to-use API.
from __future__ import annotations
from typing import List, Dict, Optional, Tuple, Any, Callable
-import json
import logging
-from pathlib import Path
from .ereader_layout import RenderingPosition, ChapterNavigator, ChapterInfo
from .page_buffer import BufferedPageRenderer
@@ -20,6 +18,9 @@ from pyWebLayout.concrete.image import RenderableImage
from pyWebLayout.style.page_style import PageStyle
from pyWebLayout.style.fonts import BundledFont
from pyWebLayout.layout.document_layouter import image_layouter
+from pyWebLayout.core.highlight import Highlight, HighlightColor, HighlightManager, \
+ create_highlight_from_query_result
+from pyWebLayout.core.persistence import ensure_dir, read_json, write_json
logger = logging.getLogger(__name__)
@@ -38,8 +39,7 @@ class BookmarkManager:
bookmarks_dir: Directory to store bookmark files
"""
self.document_id = document_id
- self.bookmarks_dir = Path(bookmarks_dir)
- self.bookmarks_dir.mkdir(exist_ok=True)
+ self.bookmarks_dir = ensure_dir(bookmarks_dir)
self.bookmarks_file = self.bookmarks_dir / f"{document_id}_bookmarks.json"
self.position_file = self.bookmarks_dir / f"{document_id}_position.json"
@@ -49,29 +49,23 @@ class BookmarkManager:
def _load_bookmarks(self):
"""Load bookmarks from file"""
- if self.bookmarks_file.exists():
- try:
- with open(self.bookmarks_file, 'r') as f:
- data = json.load(f)
- self._bookmarks = {
- name: RenderingPosition.from_dict(pos_data)
- for name, pos_data in data.items()
- }
- except Exception as e:
- print(f"Failed to load bookmarks: {e}")
- self._bookmarks = {}
+ data = read_json(self.bookmarks_file, {})
+ try:
+ self._bookmarks = {
+ name: RenderingPosition.from_dict(pos_data)
+ for name, pos_data in data.items()
+ }
+ except (AttributeError, TypeError, KeyError):
+ logger.warning("Bookmark file %s is not in the expected shape; ignoring it",
+ self.bookmarks_file, exc_info=True)
+ self._bookmarks = {}
def _save_bookmarks(self):
"""Save bookmarks to file"""
- try:
- data = {
- name: position.to_dict()
- for name, position in self._bookmarks.items()
- }
- with open(self.bookmarks_file, 'w') as f:
- json.dump(data, f, indent=2)
- except Exception as e:
- print(f"Failed to save bookmarks: {e}")
+ write_json(self.bookmarks_file, {
+ name: position.to_dict()
+ for name, position in self._bookmarks.items()
+ })
def add_bookmark(self, name: str, position: RenderingPosition):
"""
@@ -128,11 +122,7 @@ class BookmarkManager:
Args:
position: Current reading position
"""
- try:
- with open(self.position_file, 'w') as f:
- json.dump(position.to_dict(), f, indent=2)
- except Exception as e:
- print(f"Failed to save reading position: {e}")
+ write_json(self.position_file, position.to_dict())
def load_reading_position(self) -> Optional[RenderingPosition]:
"""
@@ -141,14 +131,15 @@ class BookmarkManager:
Returns:
Last reading position or None if not found
"""
- if self.position_file.exists():
- try:
- with open(self.position_file, 'r') as f:
- data = json.load(f)
- return RenderingPosition.from_dict(data)
- except Exception as e:
- print(f"Failed to load reading position: {e}")
- return None
+ data = read_json(self.position_file, None)
+ if data is None:
+ return None
+ try:
+ return RenderingPosition.from_dict(data)
+ except (TypeError, KeyError):
+ logger.warning("Position file %s is not in the expected shape; ignoring it",
+ self.position_file, exc_info=True)
+ return None
class EreaderLayoutManager:
@@ -171,7 +162,8 @@ class EreaderLayoutManager:
document_id: str = "default",
buffer_size: int = 5,
page_style: Optional[PageStyle] = None,
- bookmarks_dir: str = "bookmarks"):
+ bookmarks_dir: str = "bookmarks",
+ highlights_dir: Optional[str] = None):
"""
Initialize the ereader layout manager.
@@ -182,6 +174,8 @@ class EreaderLayoutManager:
buffer_size: Number of pages to cache in each direction
page_style: Custom page styling (uses default if None)
bookmarks_dir: Directory to store bookmark files
+ highlights_dir: Directory to store highlights. Defaults to
+ bookmarks_dir, so a document's reading state lives in one place.
"""
self.blocks = blocks
self.page_size = page_size
@@ -196,6 +190,8 @@ class EreaderLayoutManager:
self.renderer = BufferedPageRenderer(blocks, page_style, buffer_size, page_size)
self.chapter_navigator = ChapterNavigator(blocks)
self.bookmark_manager = BookmarkManager(document_id, bookmarks_dir)
+ self.highlight_manager = HighlightManager(
+ document_id, highlights_dir if highlights_dir is not None else bookmarks_dir)
# Current state
self.current_position = RenderingPosition()
@@ -848,6 +844,104 @@ class EreaderLayoutManager:
"""
return self.bookmark_manager.list_bookmarks()
+ # ------------------------------------------------------------------
+ # Highlights
+ #
+ # A Highlight carries pixel bounds, which belong to the one rendering it
+ # was taken from: change the font scale or page size and they no longer
+ # describe anything. Each highlight therefore also records the
+ # RenderingPosition of the page it was made on, and page association goes
+ # through that rather than through the bounds.
+ # ------------------------------------------------------------------
+
+ def highlight_point(self,
+ point: Tuple[int, int],
+ color: Tuple[int, int, int, int] = HighlightColor.YELLOW.value,
+ note: Optional[str] = None,
+ tags: Optional[List[str]] = None) -> Optional[Highlight]:
+ """
+ Highlight whatever is at a point on the current page.
+
+ Args:
+ point: (x, y) in page coordinates, as delivered by a tap
+ color: RGBA fill, e.g. one of HighlightColor
+ note: Optional annotation
+ tags: Optional categorization tags
+
+ Returns:
+ The stored Highlight, or None if nothing was at that point.
+ """
+ result = self.get_current_page().query_point(point)
+ if result is None or result.object_type == "empty":
+ return None
+
+ return self._store_highlight(result, color, note, tags)
+
+ def highlight_range(self,
+ start: Tuple[int, int],
+ end: Tuple[int, int],
+ color: Tuple[int, int, int, int] = HighlightColor.YELLOW.value,
+ note: Optional[str] = None,
+ tags: Optional[List[str]] = None) -> Optional[Highlight]:
+ """
+ Highlight the text between two points on the current page.
+
+ Args:
+ start: (x, y) where the selection began
+ end: (x, y) where the selection ended
+ color: RGBA fill, e.g. one of HighlightColor
+ note: Optional annotation
+ tags: Optional categorization tags
+
+ Returns:
+ The stored Highlight, or None if the range selected no text.
+ """
+ selection = self.get_current_page().query_range(start, end)
+ if not selection.results:
+ return None
+
+ return self._store_highlight(selection, color, note, tags)
+
+ def _store_highlight(self, result, color, note, tags) -> Highlight:
+ """Build a Highlight from a query result and persist it."""
+ highlight = create_highlight_from_query_result(
+ result, color=color, note=note, tags=tags,
+ position=self.current_position.to_dict())
+ self.highlight_manager.add_highlight(highlight)
+ return highlight
+
+ def remove_highlight(self, highlight_id: str) -> bool:
+ """
+ Remove a highlight.
+
+ Args:
+ highlight_id: ID of the highlight to remove
+
+ Returns:
+ True if it existed and was removed
+ """
+ return self.highlight_manager.remove_highlight(highlight_id)
+
+ def list_highlights(self) -> List[Highlight]:
+ """Get every highlight in this document."""
+ return self.highlight_manager.list_highlights()
+
+ def get_highlights_for_current_page(self) -> List[Highlight]:
+ """
+ Get the highlights made on the page currently being displayed.
+
+ Matched on the recorded RenderingPosition, so this stays correct across
+ font changes; highlights saved before the position field existed have
+ no position and are never matched.
+ """
+ current = self.current_position.to_dict()
+ return [h for h in self.highlight_manager.list_highlights()
+ if h.position == current]
+
+ def clear_highlights(self) -> None:
+ """Remove every highlight in this document."""
+ self.highlight_manager.clear_all()
+
def get_reading_progress(self) -> float:
"""
Get reading progress as a percentage.
diff --git a/tests/layout/test_ereader_highlights.py b/tests/layout/test_ereader_highlights.py
new file mode 100644
index 0000000..d6630ac
--- /dev/null
+++ b/tests/layout/test_ereader_highlights.py
@@ -0,0 +1,168 @@
+"""
+Tests for the highlight API on EreaderLayoutManager (R7).
+
+core/highlight.py was fully implemented and tested but unreachable: the manager
+had no highlight API, so highlighting could not be used through the library's
+own interface. These tests cover the wiring, not the dataclass - that is
+tests/core/test_highlight.py.
+"""
+
+import pytest
+
+from pyWebLayout.core.highlight import Highlight, HighlightColor
+from pyWebLayout.io.readers.html_extraction import parse_html_string
+from pyWebLayout.layout.ereader_manager import EreaderLayoutManager
+
+
+@pytest.fixture
+def manager(tmp_path):
+ blocks = parse_html_string(
+ "" + " ".join(f"word{i}" for i in range(300)) + "
")
+ manager = EreaderLayoutManager(blocks, page_size=(400, 600),
+ document_id="highlights",
+ bookmarks_dir=str(tmp_path))
+ yield manager
+ manager.shutdown()
+
+
+def text_points(page, limit=None):
+ """Points on the rendered page that land on a text object."""
+ found = []
+ for y in range(0, 120, 2):
+ for x in range(0, 400, 2):
+ result = page.query_point((x, y))
+ if result is not None and result.object_type == "text" and result.text:
+ found.append((x, y))
+ if limit and len(found) >= limit:
+ return found
+ return found
+
+
+@pytest.fixture
+def point_on_text(manager):
+ page = manager.get_current_page()
+ page.render()
+ return text_points(page, limit=1)[0]
+
+
+class TestHighlightPoint:
+ def test_highlighting_a_word_returns_a_stored_highlight(self, manager, point_on_text):
+ highlight = manager.highlight_point(point_on_text)
+
+ assert isinstance(highlight, Highlight)
+ assert highlight.text
+ assert manager.list_highlights() == [highlight]
+
+ def test_colour_note_and_tags_are_kept(self, manager, point_on_text):
+ highlight = manager.highlight_point(
+ point_on_text, color=HighlightColor.GREEN.value,
+ note="a note", tags=["review"])
+
+ assert highlight.color == HighlightColor.GREEN.value
+ assert highlight.note == "a note"
+ assert highlight.tags == ["review"]
+
+ def test_highlighting_empty_space_returns_none(self, manager):
+ manager.get_current_page().render()
+
+ assert manager.highlight_point((399, 599)) is None
+ assert manager.list_highlights() == []
+
+ def test_the_originating_position_is_recorded(self, manager, point_on_text):
+ highlight = manager.highlight_point(point_on_text)
+
+ assert highlight.position == manager.current_position.to_dict()
+
+
+class TestHighlightRange:
+ def test_a_selection_spans_multiple_words(self, manager):
+ page = manager.get_current_page()
+ page.render()
+ points = text_points(page)
+
+ highlight = manager.highlight_range(points[0], points[-1])
+
+ assert highlight is not None
+ assert len(highlight.text.split()) > 1
+ assert len(highlight.bounds) > 1
+
+ def test_a_selection_hitting_no_text_returns_none(self, manager):
+ manager.get_current_page().render()
+
+ assert manager.highlight_range((398, 596), (399, 599)) is None
+
+
+class TestHighlightsAreScopedToTheirPage:
+ def test_current_page_highlights_do_not_leak_across_pages(self, manager, point_on_text):
+ manager.highlight_point(point_on_text)
+ assert len(manager.get_highlights_for_current_page()) == 1
+
+ manager.next_page()
+
+ assert manager.get_highlights_for_current_page() == []
+ assert len(manager.list_highlights()) == 1, "still in the document, just not here"
+
+ def test_returning_to_the_page_finds_it_again(self, manager, point_on_text):
+ highlight = manager.highlight_point(point_on_text)
+ manager.next_page()
+ manager.previous_page()
+
+ assert manager.get_highlights_for_current_page() == [highlight]
+
+
+class TestPersistence:
+ def test_highlights_survive_a_restart(self, manager, point_on_text, tmp_path):
+ highlight = manager.highlight_point(point_on_text, note="kept")
+ manager.shutdown()
+
+ reopened = EreaderLayoutManager(
+ manager.blocks, page_size=(400, 600), document_id="highlights",
+ bookmarks_dir=str(tmp_path))
+ try:
+ restored = reopened.list_highlights()
+ assert len(restored) == 1
+ assert restored[0].id == highlight.id
+ assert restored[0].note == "kept"
+ assert restored[0].position == highlight.position
+ finally:
+ reopened.shutdown()
+
+ def test_highlights_share_the_bookmarks_directory_by_default(self, manager,
+ point_on_text, tmp_path):
+ manager.highlight_point(point_on_text)
+
+ assert (tmp_path / "highlights_highlights.json").exists()
+
+ def test_removing_a_highlight_persists(self, manager, point_on_text, tmp_path):
+ highlight = manager.highlight_point(point_on_text)
+
+ assert manager.remove_highlight(highlight.id) is True
+ assert manager.remove_highlight(highlight.id) is False
+
+ reopened = EreaderLayoutManager(
+ manager.blocks, page_size=(400, 600), document_id="highlights",
+ bookmarks_dir=str(tmp_path))
+ try:
+ assert reopened.list_highlights() == []
+ finally:
+ reopened.shutdown()
+
+ def test_clear_removes_everything(self, manager, point_on_text):
+ manager.highlight_point(point_on_text)
+
+ manager.clear_highlights()
+
+ assert manager.list_highlights() == []
+
+ def test_a_corrupt_store_does_not_stop_the_book_opening(self, tmp_path):
+ (tmp_path / "broken_highlights.json").write_text("{not json")
+ blocks = parse_html_string("hello world
")
+
+ manager = EreaderLayoutManager(blocks, page_size=(400, 600),
+ document_id="broken",
+ bookmarks_dir=str(tmp_path))
+ try:
+ assert manager.list_highlights() == []
+ assert manager.get_current_page() is not None
+ finally:
+ manager.shutdown()