Review Request 129231: [WIP] allow control over adding to "files/open recent" menu and use it in the patchreview plugin to prevent pollution

[René J.V. Bertin]

> On Oct. 21, 2016, 3:08 p.m., Kevin Funk wrote:
> > Please rebase the patch first, something's wrong here.
> > 
> > `PatchReviewToolView::open` does not exist.

You made me doubt for a second ... I have `v5.0.2-220-g766eaef` checked out (master branch), and that method is definitely there before I apply any of my patches.

Can we settle on the implementation for the master branch first and then see how it gets backported to the 5.0 branch? David's "much-needed make-over" of the patch review plugin has been working fine for me, so AFAIC it can be applied to the 5.0 branch too.


> On Oct. 21, 2016, 3:08 p.m., Kevin Funk wrote:
> > shell/documentcontroller.h, line 84
> > <https://git.reviewboard.kde.org/r/129231/diff/4/?file=482770#file482770line84>
> >
> >     This is not valid doxygen syntax. Just use `DocumentController::openDocument`, doxygen will link it

I'll presume you mean the \a opcodes. I'd only checked it through the documentation context browser/popup.


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129231/#review100199
-----------------------------------------------------------


On Oct. 21, 2016, 2:53 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129231/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2016, 2:53 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Bugs: 371210
>     http://bugs.kde.org/show_bug.cgi?id=371210
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> I've recently started noticing that my recent files menu got populated with files opened automatically by the patchreview plugin. That can lead to surprises if one of the patches you review opens files changed sufficiently long ago. What's more, the review files themselves, the temporary patch files, also end up in the menu.
> 
> The former is a more or less minor aesthetic issue, the latter is something I consider a bug; cf. the linked bug report.
> 
> The patch proposed here provides a mechanism to control whether or not files are added to the Files/Open Recent menu. Rather than adding a flag to all *DocumentController methods that might lead to adding a file to that menu (= not only the openDocument methods) I've opted for an approach with a state variable and a setter function that returns the previous state. I've kept `IDocumentController` purely abstract, so the actual implementation including the state member variable is provided by the `DocumentController` class.
> I think this approach should also maintain ABI compatibility.
> 
> The mechanism is put to use in the patch review plugin to disable updating of the recent files menu in the 2 places where files are opened automatically.
> 
> The patch also improves the (somewhat related) maximum number of documents to open feature which I think never worked as intended.
> 
> 
> Diffs
> -----
> 
>   interfaces/idocumentcontroller.h b8a41f0 
>   plugins/patchreview/patchreview.cpp 1ddec7f 
>   plugins/patchreview/patchreviewtoolview.cpp de52800 
>   shell/documentcontroller.h 35c2057 
>   shell/documentcontroller.cpp c1e9c4c 
> 
> Diff: https://git.reviewboard.kde.org/r/129231/diff/
> 
> 
> Testing
> -------
> 
> With this patch in place the patch review no longer adds all documents it opens to the Files/Open Recent menu, but this concerns only the patchfile itself and the files opened automatically and initially. Any action to open a file by the user or even activate one of the already open files still leads to adding that file to the menu, as one would expect.
> 
> Currently tested on OS X only but there is no reason this would work differently elsewhere.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161021/94872f0f/attachment.html>