Review Request 123707: Set session manager discard command

Stefan Becker chemobejk at gmail.com
Sun May 10 14:41:17 UTC 2015



> On May 9, 2015, 7:52 p.m., David Faure wrote:
> > src/core/kconfig.h, line 368
> > <https://git.reviewboard.kde.org/r/123707/diff/1/?file=367996#file367996line368>
> >
> >     I don't think this API is good to have (which is why it was never added). Most kconfig instances are a view over multiple files on disk, merged.
> >     So a single full path isn't enough.
> >     (backend->filePath() can be relative, too; this method could forward that, but then it would need to be properly documented what will be returned here, and that would be "sometimes relative, sometimes absolute"... not really convenient to work with).
> >     
> >     You can use this instead, right? QStandardPaths::writableLocation(GenericConfigLocation) + config.fileName()
> >     
> >     (If you end up keeping this method, remove "application" from the docu, this is for any KConfig object, not just the main config.
> >     and add @since 5.11.)

I thought I can't make this assumption, because it would hard-code what KConfigBackend is used and how is implemented.

Now the code checks if KConfigBackend has set a local file name or not. Please note that filePath() != name(). As far as I can tell filePath() is always relative.

Unfortunately KConfigGui is not a class so I can't mark filePath() as protected interface.

Would it be acceptable to declare kxmlgui KMainWindow as friend of KConfig, mark filePath() as protected and then move the discard command generation to kxmlgui (see also /r/123706)? Then QSessionManager can also stay "const".


- Stefan


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


On May 10, 2015, 10:47 a.m., Stefan Becker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123707/
> -----------------------------------------------------------
> 
> (Updated May 10, 2015, 10:47 a.m.)
> 
> 
> Review request for KDE Frameworks and Rex Dieter.
> 
> 
> Bugs: 346768
>     https://bugs.kde.org/show_bug.cgi?id=346768
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> If KConfig object provides a non-empty file path then set the session manager discard command to "rm <file path>". This makes sure that ksmserver removes old obsolete session files after storing a new session.
> 
> 
> Diffs
> -----
> 
>   src/core/kconfig.h 6cc7323 
>   src/core/kconfig.cpp 3819716 
>   src/gui/kconfiggui.h 173400f 
>   src/gui/kconfiggui.cpp 0048c60 
> 
> Diff: https://git.reviewboard.kde.org/r/123707/diff/
> 
> 
> Testing
> -------
> 
> F22 kwrite & konsole, ksmserver and "Save Session..."
> 
> 
> Thanks,
> 
> Stefan Becker
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150510/9696865e/attachment.html>


More information about the Kde-frameworks-devel mailing list