From kde-panel-devel Sun Jul 31 12:42:58 2016 From: Eike Hein Date: Sun, 31 Jul 2016 12:42:58 +0000 To: kde-panel-devel Subject: Re: Review Request 128552: Distinguish symlinks from other files in folder view plasmoid Message-Id: <20160731124258.20873.88106 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=146996899424498 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============8446576691993110175==" --===============8446576691993110175== Content-Type: multipart/alternative; boundary="===============1099611434434829389==" --===============1099611434434829389== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128552/#review97953 ----------------------------------------------------------- containments/desktop/package/contents/ui/FolderItemDelegate.qml (line 305) This approach is kind of wrong because return values of method calls are not notifyable props. If the file turns into a symlink or from a symlink into a not-symlink, this prop binding will have no way to know. It might work in practice because KDirModel will in most cases probably do remove+insert or model resets, but it's not correct to write code against this assumptions. My advice is whether the KDirModel::Type model column correctly identifies symlinks as such, then add a new bool data role IsSymlink or similar to FolderModel and use that in the view code. - Eike Hein On July 29, 2016, 3:40 p.m., Chinmoy Ranjan Pradhan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128552/ > ----------------------------------------------------------- > > (Updated July 29, 2016, 3:40 p.m.) > > > Review request for Plasma, Kai Uwe Broulik, Bhushan Shah, and Eike Hein. > > > Repository: plasma-desktop > > > Description > ------- > > In folder view plasmoid symlinks and ordinary files look similar. This patch makes the symlink look different by italicising its name and adding an icon overlay. > > > Diffs > ----- > > containments/desktop/package/contents/ui/FolderItemDelegate.qml e4fcd67 > containments/desktop/plugins/folder/CMakeLists.txt 1095f81 > containments/desktop/plugins/folder/foldermodel.h a6992bb > containments/desktop/plugins/folder/foldermodel.cpp 2b9d41b > > Diff: https://git.reviewboard.kde.org/r/128552/diff/ > > > Testing > ------- > > build > > > File Attachments > ---------------- > > symlinks and other files/folders look similar > https://git.reviewboard.kde.org/media/uploaded/files/2016/07/29/7428b23d-03f9-4aed-8ca0-536d44e45e8c__beforepatch.png > after patch , everything looks fine > https://git.reviewboard.kde.org/media/uploaded/files/2016/07/29/040b2e3b-9ea0-4347-9e6c-ca3bdb73b36a__link.png > > > Thanks, > > Chinmoy Ranjan Pradhan > > --===============1099611434434829389== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128552/

containments/desktop/package/contents/ui/FolderItemDelegate.qml (Diff revision 2) 

    305
    
                        font.italic: {

  


 




 


  
This approach is kind of wrong because return values of method calls are not notifyable props. If the file turns into a symlink or from a symlink into a not-symlink, this prop binding will have no way to know. It might work in practice because KDirModel will in most cases probably do remove+insert or model resets, but it's not correct to write code against this assumptions.


My advice is whether the KDirModel::Type model column correctly identifies symlinks as such, then add a new bool data role IsSymlink or similar to FolderModel and use that in the view code.

 









- Eike Hein






On July 29th, 2016, 3:40 p.m. UTC, Chinmoy Ranjan Pradhan wrote:











 

  


Review request for Plasma, Kai Uwe Broulik, Bhushan Shah, and Eike Hein.


By Chinmoy Ranjan Pradhan.




Updated July 29, 2016, 3:40 p.m.












 Repository: 
plasma-desktop





Description 

 

 

  
   
In folder view plasmoid symlinks and ordinary files look similar. This patch makes the symlink look different by italicising its name and adding an icon overlay.

  		
 






Testing 



 

  
   
build

  		
 






Diffs 


    

 
  • containments/desktop/package/contents/ui/FolderItemDelegate.qml (e4fcd67)
    • 

 
  • containments/desktop/plugins/folder/CMakeLists.txt (1095f81)
    • 

 
  • containments/desktop/plugins/folder/foldermodel.h (a6992bb)
    • 

 
  • containments/desktop/plugins/folder/foldermodel.cpp (2b9d41b)
    • 





View Diff





File Attachments 



 
  • symlinks and other files/folders look similar
    • 

 
  • after patch , everything looks fine
    • 






      		
 
    

    







  
    
 



--===============1099611434434829389==--

--===============8446576691993110175==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Disposition: inline

X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl
dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y
Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo=

--===============8446576691993110175==--