Add range selection for pages #862

bilgetutak · 2023-05-07T23:19:53Z

As discussed in #861
Added RangeSelectDialog class to handle the range selection.

Added menu item to select menu and select context menus.
Added range_select_dialog method to pdfarranger.py for completing the range selection.

codecov-commenter · 2023-05-07T23:47:59Z

Codecov Report

Attention: Patch coverage is 8.69565% with 63 lines in your changes are missing coverage. Please review.

Project coverage is 78.69%. Comparing base (13125ec) to head (16103d6).
Report is 8 commits behind head on main.

Files	Patch %	Lines
pdfarranger/pageutils.py	13.51%	32 Missing ⚠️
pdfarranger/pdfarranger.py	3.12%	31 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
- Coverage   79.62%   78.69%   -0.94%     
==========================================
  Files          13       13              
  Lines        4962     5031      +69     
==========================================
+ Hits         3951     3959       +8     
- Misses       1011     1072      +61

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kbengs

There are exceptions if you write a page number that does not exist. Also if you write a character.

But other than that it appears to work well. I suggest you wait for comments from other reviewers before doing changes.

kbengs · 2023-05-12T16:49:43Z

pdfarranger/pdfarranger.py

+                row = model[page-1]
+                self.iconview.select_path(row.path)
+
+        with self.render_lock():


The code lines 2504-2513 can be replaced with:

if range_selected is not None: self.update_statusbar()

(the rest is not needed here)

kbengs · 2023-05-12T16:50:18Z

pdfarranger/pageutils.py

+        self.selection = selection
+
+    def run_get(self):
+        """ Open the dialog and return the crop value """


pdfarranger/pageutils.py

kbengs · 2023-05-12T16:50:56Z

pdfarranger/pageutils.py

+        label.set_max_width_chars(38)
+        grid.attach(label, 0, 0, 2, 1)
+
+        label = Gtk.Label(label="Select range of pages")


This need to be translatable too

pdfarranger/pageutils.py

kbengs · 2023-05-22T16:40:23Z

Can you fix what was commented here? Also good if you can rebase/squash/fixup so all is in one commit.

dreua · 2024-02-11T14:41:50Z

@bilgetutak are you interested in following up here? Is anyone else? Otherwise I propose to copy this branch to our repo for further reference and close this stale pr.

bilgetutak · 2024-02-11T17:17:21Z

I am sorry @dreua. I was in a long project commitment, just delivered. I will work on it this week, if not I will close. Thanks for reminding.

the range selection. - Added menu item to select menu and select context menus. - Added range_select_dialog method to pdfarranger.py for completing the range selection. - Handles ranges with '-' or comma separated list of values - Handles min/max cases. (-3 ==> 0 to 3, or 4- ==> 4 to end) - Dialog has real time check of user input to limited set of characters ('0123456789,- ') using on_change signal. - Labels/titles are translatable.

bilgetutak · 2024-02-12T18:58:01Z

I have reviewed the comments here, and I believe I fixed them all. All changes in one commit this time (reversed my original fork).

kbengs · 2024-02-13T18:08:44Z

Appears to work. A few comments on what I notice:

You should use self.range_entry_widget.set_activates_default(True) Then Enter will close the dialog.
,,,,,, and ---- causes issues

Maybe change the label to:

        label=_(
                'Use a comma to separate page numbers, '
                'a dash to select a range of pages. \n'
                'e.g. : "1,3,5-7,9"'
            )

(I added a missing space and remove 'Page range to select.') And place the label under the entry instead of above? (not sure)

The selection variable is not used (I think it's fine that you unselect all before selecting the range)
The variables action, option, _unknown are not used?

(,,,,, and ----) in the selection dialog during the text entry - Removed unused variables - Adjusted labels and buttons in dialog.

bilgetutak · 2024-02-15T20:24:18Z

Appears to work. A few comments on what I notice:
You should use self.range_entry_widget.set_activates_default(True) Then Enter will close the dialog.

,,,,,, and ---- causes issues
Maybe change the label to:
        label=_(
                'Use a comma to separate page numbers, '
                'a dash to select a range of pages. \n'
                'e.g. : "1,3,5-7,9"'
            )
(I added a missing space and remove 'Page range to select.') And place the label under the entry instead of above? (not sure)

The selection variable is not used (I think it's fine that you unselect all before selecting the range)

The variables action, option, _unknown are not used?

I think most of them are fixed. Except user can enter really weird selection cases like ,-, or -,- or , ,. Most of these don't really do any selection, or select the entire model.
I tried to prevent typing any of the special characters twice (probably possible through copy-paste though).
I intended to implement an additive range selection (add on top of previous selection), but not sure if it is useful. I had the selection variable because of that. Currently removed it. In the future might be added.
Not sure about the action, option, _unknown, I kept them for compatibility with all the other functions. Not really used otherwise. Maybe for tests, when implemented?

kbengs · 2024-02-18T06:17:19Z

Not sure about the action, option, _unknown, I kept them for compatibility with all the other functions. Not really used otherwise. Maybe for tests, when implemented?

You should remove them here, as they are not used. They can be added later if needed. Can you also remove # self.set_activates_default(True) Then rebase and fixup all in one commit again. Then I think it's all good. :-)

the range selection. - Added menu item to select menu and select context menus. - Added range_select_dialog method to pdfarranger.py for completing the range selection. - Handles ranges with '-' or comma separated list of values - Handles min/max cases. (-3 ==> 0 to 3, or 4- ==> 4 to end) - Dialog has real time check of user input to limited set of characters ('0123456789,- ') using on_change signal. - Labels/titles are translatable.

kbengs · 2024-02-27T12:47:49Z

@bilgetutak it is currently in a state so that it can not be merged. The commits would need to be moved to top of the main branch. And ideally in one commit. If you do not want to/can't fix it I can do it.

bilgetutak · 2024-02-27T13:56:41Z

@kbengs I actually thought that I did reduce it in to one commit, but I didn't have experience with it before and I think I had trouble (gotta get better with git :)). If not much trouble, can you complete it. Much appreciated.

luzpaz · 2024-04-19T20:58:01Z

soft bump 🥲

kbengs reviewed May 12, 2023

View reviewed changes

dreua marked this pull request as draft July 16, 2023 11:25

dreua added need more info and removed need more info labels Jul 16, 2023

bilgetutak closed this Feb 12, 2024

bilgetutak force-pushed the main branch from 9c68550 to a73967f Compare February 12, 2024 18:05

bilgetutak reopened this Feb 12, 2024

bilgetutak marked this pull request as ready for review February 12, 2024 18:59

dreua changed the title ~~Add range selection for pages.~~ Add range selection for pages Feb 12, 2024

Fix trailing white spaces

196d461

bilgetutak added 2 commits February 15, 2024 23:13

- Fixed multiple repeating comma and dash

baf25e9

(,,,,, and ----) in the selection dialog during the text entry - Removed unused variables - Adjusted labels and buttons in dialog.

fix trailing white spaces.

624767f

bilgetutak force-pushed the main branch 2 times, most recently from 7295cc4 to 624767f Compare February 19, 2024 10:19

bilgetutak and others added 3 commits February 19, 2024 13:28

Merge branch 'pdfarranger:main' into main

9c59049

Merge branch 'range_select' into main

16103d6

kbengs mentioned this pull request Feb 27, 2024

Add range selection for pages #1065

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add range selection for pages #862

Add range selection for pages #862

bilgetutak commented May 7, 2023 •

edited

codecov-commenter commented May 7, 2023 •

edited by codecov bot

kbengs left a comment

kbengs May 12, 2023

kbengs May 12, 2023

kbengs May 12, 2023

kbengs commented May 22, 2023

dreua commented Feb 11, 2024

bilgetutak commented Feb 11, 2024

bilgetutak commented Feb 12, 2024

kbengs commented Feb 13, 2024

bilgetutak commented Feb 15, 2024 •

edited

kbengs commented Feb 18, 2024

kbengs commented Feb 27, 2024

bilgetutak commented Feb 27, 2024

luzpaz commented Apr 19, 2024

Add range selection for pages #862

Are you sure you want to change the base?

Add range selection for pages #862

Conversation

bilgetutak commented May 7, 2023 • edited

codecov-commenter commented May 7, 2023 • edited by codecov bot

Codecov Report

kbengs left a comment

Choose a reason for hiding this comment

kbengs May 12, 2023

Choose a reason for hiding this comment

kbengs May 12, 2023

Choose a reason for hiding this comment

kbengs May 12, 2023

Choose a reason for hiding this comment

kbengs commented May 22, 2023

dreua commented Feb 11, 2024

bilgetutak commented Feb 11, 2024

bilgetutak commented Feb 12, 2024

kbengs commented Feb 13, 2024

bilgetutak commented Feb 15, 2024 • edited

kbengs commented Feb 18, 2024

kbengs commented Feb 27, 2024

bilgetutak commented Feb 27, 2024

luzpaz commented Apr 19, 2024

bilgetutak commented May 7, 2023 •

edited

codecov-commenter commented May 7, 2023 •

edited by codecov bot

bilgetutak commented Feb 15, 2024 •

edited