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

[Aleix Pol Gonzalez]

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




plugins/patchreview/patchreview.cpp (line 469)
<https://git.reviewboard.kde.org/r/129231/#comment67272>

    Why's that change?



shell/documentcontroller.h (line 120)
<https://git.reviewboard.kde.org/r/129231/#comment67273>

    I'm not very fond of adding a new state for the DocumentController. Maybe it would make sense to add an argument to openDocument that specifies if it should be added to the recent files list?


- Aleix Pol Gonzalez


On Oct. 20, 2016, 10:27 a.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. 20, 2016, 10:27 a.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/20161020/d7753401/attachment-0001.html>