Review Request 109986: Documents sidebar categorization by path

[Andreas Pakulat]

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



plugins/documentview/kdevdocumentview.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23004>

    This and the above three lines could be done more safely with QFileInfo(label).path() (and also shorter). Even though creating a QFileInfo incurs an overhead, this shouldn't be that much of an issue since one rarely opens new documents within milliseconds.



plugins/documentview/kdevdocumentview.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23003>

    Just minor, but the variable name does not fit well anymore



plugins/documentview/kdevdocumentview.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23005>

    I think it would be nicer to have a dedicated getter for the label. If in the future we decide to include more data in the tooltip this will suddenly break.



plugins/documentview/kdevdocumentview.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23006>

    You could store the projects instead of the project folders and the urls in the categories instead of just strings.
    
    This will improve the type-safety, allow to use this stuff easily with remote-projects and let you use IProject::relativeUrl() to get a project-relative path.



plugins/documentview/kdevdocumentview.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23007>

    This is the same 3 lines as above in opened(). If you change it there change it here too and if not, at least factor out the duplicated lines into a local static function.



plugins/documentview/kdevdocumentview.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23008>

    Storing the projects instead of just their folders would also allow you to avoid the extra clearing/iteration here and just use assignment.



plugins/documentview/kdevdocumentviewdelegate.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23010>

    Whats the reason for this?



plugins/documentview/kdevdocumentviewdelegate.cpp
<http://git.reviewboard.kde.org/r/109986/#comment23009>

    Hmm, this seems to still elide the text - according to your screenshot. So I guess the elidedText function call was never needed?


In general I think the change is great.

- Andreas Pakulat


On April 12, 2013, 11:50 p.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109986/
> -----------------------------------------------------------
> 
> (Updated April 12, 2013, 11:50 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> This patch changes the categorization in KDevelop's Documents sidebar to categorizing by path, not by mimetype anymore.
> 
> The problems with sorting by mimetypes are:
> 
> - files that are related to each other are often far away in the UI
> - there are often files with the same name right underneath each other, making picking the right one almost impossible
> 
> With this patch, the sidebar is organized per path, in alphabetical order. This makes the location of a file in the UI more predictable, groups file in a more meaningful way.
> 
> 
> I've asked a few people on IRC about this, nobody was particularly attached to the current design, so I changed the current one, instead of adding a new plugin. (It makes sense to me personally as well, I don't see how categorization by mimetype there is useful, but my use case is rather limited).
> 
> Please give it a try, tell me what you think of it. You can find the code in the sebas/sidebar branch in kdevplatform.
> 
> 
> Diffs
> -----
> 
>   plugins/documentview/kdevdocumentviewdelegate.cpp b172ac0d5b2c1a60cbd45d26e30b72ef725d8678 
>   plugins/documentview/kdevdocumentview.desktop.cmake a89e79e18a9b79becd669813ead2d2e31f588a4d 
>   plugins/documentview/kdevdocumentview.cpp b8c4a02ad31fb8aaf29691d7d7d991c4b699360e 
>   plugins/documentview/kdevdocumentview.h e237f1050aee6c4ef383c809d929b524f94cb938 
>   plugins/documentview/kdevdocumentmodel.cpp b411a815680ee92d8eaf81b19266f24c6b4a4f13 
>   plugins/documentview/kdevdocumentmodel.h 05cff452c403156cd53e9caaf029d6bbcd394862 
> 
> Diff: http://git.reviewboard.kde.org/r/109986/diff/
> 
> 
> Testing
> -------
> 
> Tested with my actual projects, behaviour feels much better, I now (kind of ;)) enjoy the sidebar, rather than outright hating it. :-)
> 
> I didn't notice any particular misbehaviour or breakage, performance also doesn't seem to be a problem.
> 
> 
> File Attachments
> ----------------
> 
> original sidebar
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/04/12/kdevelop-sidebar-original.png
> new sidebar
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/04/12/kdevelop-sidebar-new.png
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130413/894a941e/attachment-0001.html>