- Group Reviewers
- R223:f788b5a384b5: Add action in Edit menu to select the text on current page
Click on "Select All Text on Current Page" entry in Edit menu to select the entire page. The selected text can then be copied via Edit menu item "Copy"
Personally i'm happy with a menu option, "no one" will see it, but that's the point, it's an option that "no one" needs.
And for the people that need it, they'll search for it, google it, maybe even try to use the documentation and then they'll find it in the menu.
And then people that for some reason need to select pages very often and are smart will learn how to assign the shortcut they want.
All right, let's do this. In final testing, I just found some blockers:
- If the current mouseMode is not TextSelect, then the menu item is enabled but does nothing. It should do its thing no matter what the mouseMode is, just like Select all does.
- The menu item is still enabled when Okular is opened without a document, and clicking on it crashes Okular. It needs to be disabled when no document is open.
No. I think that the action should be created in part.cpp, setEnabled(false) by default, then setEnabled(true) in the Part::openFile to avoid the crash.
Regretfully, no function (like isOpened) for setEnabled to connect to.
@aacid area is a RegularAreaRect object which is a QRect bounding the selected text. "area" is to be deleted in all the cases except the case in which it is not possible to delete it since "area" will not contain any text in it and hence it will be nullptr. (case: if text is empty string) and hence return is called instead of deleting the "area".
Wait, stupid question: You assume area to be nullptr if there is no text on the page?
You do area = TextSelectionForItem( item );, PageView::TextSelectionForItem() unconditionally returns a Okular::RegularAreaRect * which it got from TextPage::TextArea(). TextPage::TextArea() does return new RegularAreaRect(); if TextSelectionForItem() is called on an empty page, and RegularAreaRect * ret= new RegularAreaRect; [complicated] return ret; [or] ret->addShape([...]); return ret otherwise.
TextSelectionForItem() tests whether it gets nullptr from TextArea(). But how can TextArea() return nullptr at all? new RegularAreaRect() vs. new RegularAreaRect?
Interestingly, setPageTextSelection() deletes area (without making it nullptr). Shouldn’t this side effect be documented (in another patch)?
The other selection tools copy to clipboard. Maybe Select All should do so too, because the selection can not be used any other way than Menu -> Edit -> Copy. (Other way: e. g. right click -> Search With DuckDuckGo)
Sure if you want to create a patch that documents it, feel free (BTW it can't set the pointer to null for the caller, that's not how pointers work)
Not convinced, for example kate doesn't do that either.
open kate, type something, press ctrl+a to select it, it only gets selected, not copied to the clipboard
but if you select it with the mouse it then gets copied to the clipboard.
It'd be the same we're doing, so i think this consistency is good to maintain.