From kwrite-devel Sun Nov 10 21:27:42 2013 From: "Alex Turbov" Date: Sun, 10 Nov 2013 21:27:42 +0000 To: kwrite-devel Subject: Re: Review Request 113720: Python plugins refactored Message-Id: <20131110212742.9462.76086 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kwrite-devel&m=138411887803591 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============7487178038247290871==" --===============7487178038247290871== Content-Type: multipart/alternative; boundary="===============7191955921086201207==" --===============7191955921086201207== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Nov. 10, 2013, 11:22 a.m., Dominik Haumann wrote: > > addons/kate/pate/src/engine.cpp, line 185 > > > > > > Now that the Pate::Engine() has no parent, who deletes finally? Maybe I'm missing a delete call in the destructor of the Pate plugin or somehwere else? Going to make it as a data member of Plugin class instead of singleton. > On Nov. 10, 2013, 11:22 a.m., Dominik Haumann wrote: > > addons/kate/pate/src/engine.h, line 51 > > > > > > Since this is a public struct, we usually omit the m_ prefix in this case. And: > > - srv -> service > > - module_file -> moduleFile > > - error_reason -> errorReason Going to make it as a class w/ accessors... Only Engine can make instances of it, everybody else can only read it... - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113720/#review43330 ----------------------------------------------------------- On Nov. 8, 2013, 6:09 a.m., Alex Turbov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113720/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2013, 6:09 a.m.) > > > Review request for Kate and Shaheed Haque. > > > Repository: kate > > > Description > ------- > > Search installed Python plugins via trader. > Introduce `Kate/PythonPlugin` service type. > Make plugins management look-n-feel as C++ plugins. > Enabled plugins now are per session (stored under "Enabled Plugins" key of the pate config group), so it is possible to have different plugins in different sessions (just like C++ plugins). > > Sample .desktop file: > > [Desktop Entry] > Type=Service > ServiceTypes=Kate/PythonPlugin > X-KDE-Library=cmake_utils/cmake_utils.py > Name=CMake Utilities > Comment=Code completer, cache and help browser > X-Python-2-Compatible=false > X-Python-3-Compatible=true > > The patch contains .desktop files only for plugins I wrote (and use) myself. Other plugins still need .desktop files (I don't know exactly which Python they are require) > > > Diffs > ----- > > addons/kate/pate/src/utilities.cpp 3ed19d6 > addons/kate/pate/src/utilities.h d88af0f > addons/kate/pate/src/plugins/format.desktop PRE-CREATION > addons/kate/pate/src/plugins/expand.desktop PRE-CREATION > addons/kate/pate/src/plugins/commentar.desktop PRE-CREATION > addons/kate/pate/src/plugins/color_tools.desktop PRE-CREATION > addons/kate/pate/src/plugins/cmake_utils/cmake_utils.desktop PRE-CREATION > addons/kate/pate/src/plugins/block.desktop PRE-CREATION > addons/kate/pate/src/plugin.cpp 85c84a3 > addons/kate/pate/src/plugin.h 77369c6 > addons/kate/pate/src/manager.ui 82213ee > addons/kate/pate/src/katepythonplugin.desktop PRE-CREATION > addons/kate/pate/src/engine.cpp 5484473 > addons/kate/pate/src/engine.h 6c70ab9 > addons/kate/pate/src/CMakeLists.txt 78bbdc0 > > Diff: http://git.reviewboard.kde.org/r/113720/diff/ > > > Testing > ------- > > > File Attachments > ---------------- > > Python Plugins Management > http://git.reviewboard.kde.org/media/uploaded/files/2013/11/08/f711a85f-34f7-4b2c-99d2-f1c375fcf88f__python-plugins-redesigned.png > > > Thanks, > > Alex Turbov > > --===============7191955921086201207== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113720/

On November 10th, 2013, 11:22 a.m. UTC, Dominik Haumann wrote:

addons/kate/pate/src/engine.h (Diff revision 1) 

    46
    
        KService::Ptr m_srv;

  


 




  
Since this is a public struct, we usually omit the m_ prefix in this case. And:
- srv -> service
- module_file -> moduleFile
- error_reason -> errorReason

 









Going to make it as a class w/ accessors... Only Engine can make instances of it, everybody else can only read it...






 
On November 10th, 2013, 11:22 a.m. UTC, Dominik Haumann wrote:

 

  





 
  

   
    addons/kate/pate/src/engine.cpp
    

     (Diff revision 1)

    
   
  

 

 
  


   
PyMODINIT_FUNC PATE_INIT(void)


  

 



 
 

 

  

    181
    
        s_self = new Pate::Engine(qApp);

    110
    
        s_self = new Pate::Engine();

  


 




  
Now that the Pate::Engine() has no parent, who deletes finally? Maybe I'm missing a delete call in the destructor of the Pate plugin or somehwere else?

 









Going to make it as a data member of Plugin class instead of singleton.








- Alex






On November 8th, 2013, 6:09 a.m. UTC, Alex Turbov wrote:











 

  


Review request for Kate and Shaheed Haque.


By Alex Turbov.




Updated Nov. 8, 2013, 6:09 a.m.












 Repository: 
kate





Description 

 

 

  
   
Search installed Python plugins via trader. 
Introduce `Kate/PythonPlugin` service type. 
Make plugins management look-n-feel as C++ plugins.
Enabled plugins now are per session (stored under "Enabled Plugins" key of the pate config group), so it is possible to have different plugins in different sessions (just like C++ plugins).

Sample .desktop file:

[Desktop Entry]
Type=Service
ServiceTypes=Kate/PythonPlugin
X-KDE-Library=cmake_utils/cmake_utils.py
Name=CMake Utilities
Comment=Code completer, cache and help browser
X-Python-2-Compatible=false
X-Python-3-Compatible=true

The patch contains .desktop files only for plugins I wrote (and use) myself. Other plugins still need .desktop files (I don't know exactly which Python they are require)

  		
 







Diffs 


    

 
  • addons/kate/pate/src/utilities.cpp (3ed19d6)
    • 

 
  • addons/kate/pate/src/utilities.h (d88af0f)
    • 

 
  • addons/kate/pate/src/plugins/format.desktop (PRE-CREATION)
    • 

 
  • addons/kate/pate/src/plugins/expand.desktop (PRE-CREATION)
    • 

 
  • addons/kate/pate/src/plugins/commentar.desktop (PRE-CREATION)
    • 

 
  • addons/kate/pate/src/plugins/color_tools.desktop (PRE-CREATION)
    • 

 
  • addons/kate/pate/src/plugins/cmake_utils/cmake_utils.desktop (PRE-CREATION)
    • 

 
  • addons/kate/pate/src/plugins/block.desktop (PRE-CREATION)
    • 

 
  • addons/kate/pate/src/plugin.cpp (85c84a3)
    • 

 
  • addons/kate/pate/src/plugin.h (77369c6)
    • 

 
  • addons/kate/pate/src/manager.ui (82213ee)
    • 

 
  • addons/kate/pate/src/katepythonplugin.desktop (PRE-CREATION)
    • 

 
  • addons/kate/pate/src/engine.cpp (5484473)
    • 

 
  • addons/kate/pate/src/engine.h (6c70ab9)
    • 

 
  • addons/kate/pate/src/CMakeLists.txt (78bbdc0)
    • 





View Diff





File Attachments 








  		
 











  

 



--===============7191955921086201207==--

--===============7487178038247290871==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel

--===============7487178038247290871==--