Implement backwards navigation by pressing N

[20mattia02]

Hi, I often find myself having to scroll back and forth the results, but the program won't let me.

It would be great if you merged this into upstream, so I don't have to maintain my own patches.

If you intend to proceed, I'll update buku.1 and prompt_help accordingly.

Thank you!

[jarun]

Yes, this is useful.

Please ensure to test the corner cases like no results, total number of results exactly divisible by number of results shown per page, less than 1 page of results and so on.

[jarun]

lint check fails on CI due to this line:

./buku.py:4727:32: E225 missing whitespace around operator

Please check.

[LeXofLeviafan]

Why not use in here as well? It's literally the same condition.

[LeXofLeviafan]

When adding a new command to the interactive mode, you may want to update the "PROMPT KEYS" list (prompt_help() in the code + docfiles: README.md, buku.1)

[LeXofLeviafan]

This line should be moved to after the print loop; it'd make the code within this block much less confusing.

This would also remove the need of resetting next_index when replacing results (…so you should replace all instances of cur_index = next_index = 0 with just cur_index = 0, to simplify the code).

(Nevermind that, seems like there's logic elsewhere that depends on this particular order of actions… Though this line can still be moved out of the if.)

[LeXofLeviafan]

…You can also move the total_results line above it – that'd allow to simplify the ifs here (potentially combining them into a single chain if you flip the topmost if at least).

If you want to avoid causing errors when None is passed, you can simply add a fallback value: len(results or []).

[LeXofLeviafan]

Why not use in here as well? It's literally the same condition.

[LeXofLeviafan]

Why did you add new_results and to this condition? I can't think of any case where that would be useful 🤔

If we've reached the end of the list, cur_index will be >= total_results. And if we haven't, new_results won't be set to False.

If you had enabled navigation after reaching the end, it'd be different; but then, this additional condition would undo the effect of enabling it in the first place…

[LeXofLeviafan]

…Incidentally, if you want to enable navigation after reaching the end, you'd need an or here instead, since cur_index < total_results would prevent evaluation otherwise (e.g. or nav == 'N').

[LeXofLeviafan]

How… complicated.

How about you make a variable named prev_index and just set it to max(0, cur_index - num) before the print loop? (And instead of this whole conditional chain, you can simply set cur_index to min(cur_index, prev_index)… so that you don't need to reset prev_index when replacing results).

As for count and next_index, their values are actually not dependent on whether we're moving forwards or backwards (see below).

[LeXofLeviafan]

count is the iteration index; it should always be set to cur_index before the loop (in the previous version of the code, that was done implicitly by the iteration). Doing so shouldn't be placed within an if.

This particular assignment of next_index is also universal (regardless of your cur_index, this value simply denotes position of the end of the page being printed). Placing it withing an if makes no sense either.

[LeXofLeviafan]

BTW, I suggest flipping the condition within this and the next loop (so that the branches will be swapped).

The else: branches in both of them take up merely 2 lines, and are barely noticeable after the much larger "then" branches; placing them first would make the logic of this entire block much clearer.

[LeXofLeviafan]

Here's a patch with a simplified implementation that appears to work well

(This version allows for backtracking after reaching the end; if you don't want that, remove and nav != 'N'. Also, I removed count altogether, to avoid confusion with its purpose.)

diff --git a/buku b/buku
index d0c8466..1f80ebb 100755
--- a/buku
+++ b/buku
@@ -4660,7 +4660,7 @@ def prompt(obj, results, noninteractive=False, deep=False, listtags=False, sugge
 
     new_results = bool(results)
     nav = ''
-    cur_index = next_index = count = 0
+    cur_index = next_index = prev_index = 0
 
     if listtags:
         show_taglist(obj)
@@ -4672,44 +4672,32 @@ def prompt(obj, results, noninteractive=False, deep=False, listtags=False, sugge
 
     if noninteractive:
         try:
-            for row in results:
-                count += 1
-                print_single_rec(row, count, columns)
-        except Exception:
-            pass
+            for i, row in enumerate(results):
+                print_single_rec(row, i + 1, columns)
+        except Exception as e:
+            LOGERR(e)
         return
 
     skip_print = False
     while True:
         if (new_results or nav in ['n', 'N']) and not skip_print:
-            if results:
-                total_results = len(results)
-                cur_index = next_index
-                if new_results and cur_index < total_results:
-                    if nav == 'N':
-                        if cur_index <= num:
-                            cur_index = count = 0
-                            next_index = num
-                        elif cur_index % num != 0:
-                            next_index = cur_index - (cur_index % num)
-                            count = cur_index = next_index - num
-                        else:
-                            cur_index = count = cur_index - num * 2
-                            next_index -= num
-                    elif cur_index < total_results:
-                        count = next_index
-                        next_index = min(cur_index + num, total_results)
-                    print()
-                    for row in results[cur_index:next_index]:
-                        count += 1
-                        print_single_rec(row, count, columns)
-                    print('%d-%d/%d' % (cur_index + 1, next_index, total_results))
-                else:
-                    print('No more results')
-                    new_results = False
-            else:
+            _total_results = len(results or [])
+            cur_index = next_index              # used elsewhere as "most recent page start index"
+            if not results:
                 print('0 results')
                 new_results = False
+            elif cur_index >= _total_results and nav != 'N':
+                print('No more results')
+                new_results = False
+            else:
+                if nav == 'N':
+                    cur_index = min(cur_index, prev_index)
+                prev_index = max(0, cur_index - num)
+                next_index = min(cur_index + num, _total_results)
+                print()
+                for i in range(cur_index, next_index):
+                    print_single_rec(results[i], i + 1, columns)
+                print('%d-%d/%d' % (cur_index + 1, next_index, _total_results))
         skip_print = False
 
         try:
@@ -4724,7 +4712,7 @@ def prompt(obj, results, noninteractive=False, deep=False, listtags=False, sugge
             return
 
         # show the next set of results from previous search
-        if nav == 'n' or nav =='N':
+        if nav in ('n', 'N'):
             continue
 
         if (m := re.match(r'^R(?: (-)?([0-9]+))?$', nav.rstrip())) and (n := int(m[2] or 1)) > 0:
@@ -4865,7 +4853,7 @@ def prompt(obj, results, noninteractive=False, deep=False, listtags=False, sugge
         # Copy URL to clipboard
         if nav.startswith('c ') and nav[2:].isdigit():
             index = int(nav[2:]) - 1
-            if index < 0 or index >= count:
+            if index < 0 or index >= next_index:
                 print('No matching index')
                 continue
             copy_to_clipboard(content=results[index + cur_index][1].encode('utf-8'))
@@ -4894,7 +4882,7 @@ def prompt(obj, results, noninteractive=False, deep=False, listtags=False, sugge
         for nav in nav.split():
             if is_int(nav):
                 index = int(nav) - 1
-                if index < 0 or index >= count:
+                if index < 0 or index >= next_index:
                     print('No matching index %s' % nav)
                     continue
                 browse(results[index][1])
@@ -4905,7 +4893,7 @@ def prompt(obj, results, noninteractive=False, deep=False, listtags=False, sugge
                         vals[0], vals[-1] = vals[-1], vals[0]
 
                     for _id in range(vals[0]-1, vals[-1]):
-                        if 0 <= _id < count:
+                        if 0 <= _id < next_index:
                             browse(results[_id][1])
                         else:
                             print('No matching index %d' % (_id + 1))