Review Request: load the images for the themes correctly

[Aaron J. Seigo]

> On Nov. 29, 2012, 1:53 p.m., Aaron J. Seigo wrote:
> > plasmate/packagemodel.cpp, line 544
> > <http://git.reviewboard.kde.org/r/106680/diff/5/?file=93807#file93807line544>
> >
> >     "Plasma/Theme" should not appear in this file.
> 
> Giorgos Tsiapaliokas wrote:
>     the "colors" file isn't a requiredDirectory nor a requiredFile, so If I don't add it by hand in the m_topEntries
>     it won't be visible and also we want the "colors" file in the m_topEntries only when the package is a theme.
>     No? What am I missing?
> 
> Aaron J. Seigo wrote:
>     perhaps add an exception for top level non-required files, so that everything in the top level, required or not, is shown.
> 
> Giorgos Tsiapaliokas wrote:
>     I can't find a way (I don't think that there is one) in order to specify which files are top level ones.
>     
>     Before the refactor of the packagemodel, the model was doing this:
>     * find all the files(required or not)
>     * check if you can give them a name and call them namedFiles(remove the ones which have a name from the hash)
>     * everything that has been left in the hash add it as a top level item
>     
>     Now with the refactor of the packagemodel, it does:
>     * put the required files in a hash and all(including the required ones) the files in another one
>     ** We need the second hash with all the files, because if the project already exists, we want to
>     add in the ui(with names) all the existing files not just the required ones.
>     * add all the required files in the ui and give them a name (namedFiles)
>     * if there are requiredFiles still in the hash add them in the ui.
>     
>     I see your point that adding a hard coded key can make plasmate fail at some point,
>     but those keys doesn't change often(actually they don't change at all. No?) so we are fine(not ok).
>     Even if plasmate fails, in the -worst/and only- case scenario the user won't be able see/modify the "colors" file

Thinking about it some more, all named files should be shown. Everything that is not a directory but a key with a single file associated with it should be shown, otherwise any package structure that has a file in the same way ThemePackage has colors will fail.


- Aaron J.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106680/#review22754
-----------------------------------------------------------


On Dec. 7, 2012, 5:58 p.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106680/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2012, 5:58 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> create a new theme package->click on the new
> 
> a file dialog should appear but instead a simple edit box appears requesting a new filename.
> 
> This patch solves the issue
> 
> 
> Diffs
> -----
> 
>   plasmate/editors/editpage.h 5cb3ea6 
>   plasmate/editors/editpage.cpp 7e82ff2 
>   plasmate/packagemodel.h efa3001 
>   plasmate/packagemodel.cpp 9eb0914 
> 
> Diff: http://git.reviewboard.kde.org/r/106680/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliokas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20121221/d989ef28/attachment-0001.html>