From a0c42064776c0d8c3665cbc3ce653681852edf65 Mon Sep 17 00:00:00 2001
From: Kovid Goyal <kovid@kovidgoyal.net>
Date: Sun, 10 Aug 2014 08:28:09 +0530
Subject: [PATCH] E-book viewer: More accurate sorting of bookmarks by position
 in book

---
 src/calibre/ebooks/epub/cfi/parse.py       | 29 ++++++++++++++++++++++
 src/calibre/ebooks/epub/cfi/tests.py       | 12 ++++++++-
 src/calibre/gui2/viewer/bookmarkmanager.py |  3 ++-
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/calibre/ebooks/epub/cfi/parse.py b/src/calibre/ebooks/epub/cfi/parse.py
index ed8f82953f..fb80b9f3bb 100644
--- a/src/calibre/ebooks/epub/cfi/parse.py
+++ b/src/calibre/ebooks/epub/cfi/parse.py
@@ -167,3 +167,32 @@ def parser():
         _parser = Parser()
     return _parser
 
+def get_steps(pcfi):
+    ans = tuple(pcfi['steps'])
+    if 'redirect' in pcfi:
+        ans += get_steps(pcfi['redirect'])
+    return ans
+
+def cfi_sort_key(cfi, only_path=True):
+    p = parser()
+    try:
+        if only_path:
+            pcfi = p.parse_path(cfi)[0]
+        else:
+            parent, start = p.parse_epubcfi(cfi)[:2]
+            pcfi = start or parent
+    except Exception:
+        import traceback
+        traceback.print_exc()
+        return ()
+    if not pcfi:
+        import sys
+        print ('Failed to parse CFI: %r' % pcfi, file=sys.stderr)
+        return ()
+    steps = get_steps(pcfi)
+    step_nums = tuple(s.get('num', 0) for s in steps)
+    step = steps[-1] if steps else {}
+    offsets = (step.get('temporal_offset', 0), tuple(reversed(step.get('spatial_offset', (0, 0)))), step.get('text_offset', 0), )
+    return (step_nums, offsets)
+
+
diff --git a/src/calibre/ebooks/epub/cfi/tests.py b/src/calibre/ebooks/epub/cfi/tests.py
index f50762639d..1787ccb108 100644
--- a/src/calibre/ebooks/epub/cfi/tests.py
+++ b/src/calibre/ebooks/epub/cfi/tests.py
@@ -9,10 +9,20 @@ __copyright__ = '2014, Kovid Goyal <kovid at kovidgoyal.net>'
 import unittest
 from future_builtins import map
 
-from calibre.ebooks.epub.cfi.parse import parser
+from calibre.ebooks.epub.cfi.parse import parser, cfi_sort_key
 
 class Tests(unittest.TestCase):
 
+    def test_sorting(self):
+        null_offsets = (0, (0, 0), 0)
+        for path, key in [
+                ('/1/2/3', ((1, 2, 3), null_offsets)),
+                ('/1[id]:34[yyyy]', ((1,), (0, (0, 0), 34))),
+                ('/1@1:2', ((1,), (0, (2, 1), 0))),
+                ('/1~1.2', ((1,), (1.2, (0, 0), 0))),
+        ]:
+            self.assertEqual(cfi_sort_key(path), key)
+
     def test_parsing(self):
         p = parser()
         def step(x):
diff --git a/src/calibre/gui2/viewer/bookmarkmanager.py b/src/calibre/gui2/viewer/bookmarkmanager.py
index 84e23a8752..0f9bc4e0af 100644
--- a/src/calibre/gui2/viewer/bookmarkmanager.py
+++ b/src/calibre/gui2/viewer/bookmarkmanager.py
@@ -173,9 +173,10 @@ class BookmarkManager(QWidget):
         self.edited.emit(bm)
 
     def sort_by_pos(self):
+        from calibre.ebooks.epub.cfi.parse import cfi_sort_key
         def pos_key(b):
             if b.get('type', None) == 'cfi':
-                return b['spine'], b['pos']
+                return b['spine'], cfi_sort_key(b['pos'])
             return (None, None)
         bm = self.get_bookmarks()
         bm.sort(key=pos_key)