Re: Review Request 119678: add a target named _doc for each project in the tree, to generate developer documention

[Mathieu Tarral]

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

(Updated août 21, 2014, 10:50 après-midi)


Status
--

This change has been discarded.


Review request for KDE Base Apps.


Repository: kde-baseapps


Description
---

add a target named _doc for each project in the tree, to generate 
developer documention by extracting code structure, and creating useful call 
graphs. It's very useful to new contributors, so they can find their way into 
the source tree by exploring files and structures into their favorite browser.


Diffs
-

  CMakeLists.txt b06ba01fff7cee445fcc0b896cef041bd4c34fc8 
  cmake/in/Doxyfile.in.cmake PRE-CREATION 
  cmake/modules/doxygen.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119678/diff/


Testing
---

mkdir _build
cd _build
cmake .. -DCMAKE_INSTALL_PREFIX=../_install
make dolphin_doc
make kdialog_doc
etc ...


Thanks,

Mathieu Tarral

Re: Review Request 119678: add a target named _doc for each project in the tree, to generate developer documention

[Mathieu Tarral]

> On août 20, 2014, 8:55 après-midi, Allen Winter wrote:
> > We have a standalone program in kde-dev-scripts called kdedoxygen.sh that 
> > might accomplish the same thing.  We also have kdedoxyman.sh (for man 
> > pages) and kdedoxyqt.sh (for Qt assistant files)
> > 
> > might be worth a look.

it's a shame i never heard about kde-dev-scripts...
and yes kdedoxygen.sh is actually doing what i implemented using CMake.
Is it referenced on the KDE TechBase ???

Thanks for your comment.
i will close the review request.


- Mathieu


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


On août 9, 2014, 12:43 matin, Mathieu Tarral wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119678/
> ---
> 
> (Updated août 9, 2014, 12:43 matin)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> add a target named _doc for each project in the tree, to generate 
> developer documention by extracting code structure, and creating useful call 
> graphs. It's very useful to new contributors, so they can find their way into 
> the source tree by exploring files and structures into their favorite browser.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt b06ba01fff7cee445fcc0b896cef041bd4c34fc8 
>   cmake/in/Doxyfile.in.cmake PRE-CREATION 
>   cmake/modules/doxygen.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119678/diff/
> 
> 
> Testing
> ---
> 
> mkdir _build
> cd _build
> cmake .. -DCMAKE_INSTALL_PREFIX=../_install
> make dolphin_doc
> make kdialog_doc
> etc ...
> 
> 
> Thanks,
> 
> Mathieu Tarral
> 
>

Re: Review Request 119892: [Dolphin] Implement "Add to archive" option when dragging and dropping onto an archive file in dolphin

[Arjun Ak]

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



lib/konq/konq_operations.cpp


What should be done with this? Is this feature necessary?


- Arjun Ak


On Aug. 22, 2014, 2:41 a.m., Arjun Ak wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119892/
> ---
> 
> (Updated Aug. 22, 2014, 2:41 a.m.)
> 
> 
> Review request for KDE Base Apps and KDE Utils.
> 
> 
> Bugs: 338414
> http://bugs.kde.org/show_bug.cgi?id=338414
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> This patch implements the "Add to archive" option, which is shown when a user 
> drags and drops files onto an existing archive. 
> 
> 
> See also:
> https://git.reviewboard.kde.org/r/119890
> https://bugs.kde.org/show_bug.cgi?id=338414
> 
> 
> Diffs
> -
> 
>   dolphin/src/views/dolphinview.cpp 02b8815 
>   dolphin/src/views/draganddrophelper.cpp f8ae0ad 
>   lib/konq/konq_operations.cpp 220a90a 
> 
> Diff: https://git.reviewboard.kde.org/r/119892/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Ak
> 
>

Review Request 119892: [Dolphin] Implement "Add to archive" option when dragging and dropping onto an archive file in dolphin

[Arjun Ak]

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

Review request for KDE Base Apps and KDE Utils.


Bugs: 338414
http://bugs.kde.org/show_bug.cgi?id=338414


Repository: kde-baseapps


Description
---

This patch implements the "Add to archive" option, which is shown when a user 
drags and drops files onto an existing archive. 


See also:
https://git.reviewboard.kde.org/r/119890
https://bugs.kde.org/show_bug.cgi?id=338414


Diffs
-

  dolphin/src/views/dolphinview.cpp 02b8815 
  dolphin/src/views/draganddrophelper.cpp f8ae0ad 
  lib/konq/konq_operations.cpp 220a90a 

Diff: https://git.reviewboard.kde.org/r/119892/diff/


Testing
---


Thanks,

Arjun Ak

Review Request 119890: [Ark] Implement "Add to archive" option when dragging and dropping onto an archive file in dolphin

[Arjun Ak]

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

Review request for KDE Base Apps and KDE Utils.


Bugs: 338414
http://bugs.kde.org/show_bug.cgi?id=338414


Repository: ark


Description
---

This patch implements the "Add to archive" option, which is shown when a user 
drags and drops files onto an existing archive. 


See also:
https://git.reviewboard.kde.org/r/119892
https://bugs.kde.org/show_bug.cgi?id=338414


Diffs
-

  app/ark_dndaddtoarchive.desktop.cmake PRE-CREATION 
  app/CMakeLists.txt f1ef01b 
  app/addToArchiveDndPlugin.h PRE-CREATION 
  app/addToArchiveDndPlugin.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119890/diff/


Testing
---


Thanks,

Arjun Ak

Re: Review Request 119869: KIO: Correctly handle .directory files

[David Faure]

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

Ship it!



src/widgets/kdesktopfileactions.cpp


QLatin1String is sufficient, for comparisons


- David Faure


On Aug. 21, 2014, 1:15 p.m., Maarten De Meyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119869/
> ---
> 
> (Updated Aug. 21, 2014, 1:15 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and Frank Reininghaus.
> 
> 
> Bugs: 235457
> https://bugs.kde.org/show_bug.cgi?id=235457
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> My attempt at fixing [bug 235457](https://bugs.kde.org/show_bug.cgi?id=235457)
> We give .directory the default Type 'Directory' and a way for 
> KDesktopFileActions to handle .directory files.
> To do that we trick KRun in opening the file as plain text.
> 
> hasDirectoryType is a new method in KConfig. see: 
> [119868](https://git.reviewboard.kde.org/r/119868/) 
> Anything I should look out for when commiting this to KConfig and KIO?
> 
> 
> Diffs
> -
> 
>   src/widgets/kdesktopfileactions.cpp 9486015 
> 
> Diff: https://git.reviewboard.kde.org/r/119869/diff/
> 
> 
> Testing
> ---
> 
> Configure a folder with a custom icon in dolphin. Click the created 
> .directory file.
> 
> 
> Thanks,
> 
> Maarten De Meyer
> 
>

Re: Review Request 119875: add support for Clang Static Analyzer and improve build configuration

[Albert Astals Cid]

> On ago. 20, 2014, 10:15 p.m., Albert Astals Cid wrote:
> > Why is this per repo? This should be in a place all repos get the nice 
> > stuff without having to do work.
> > 
> > A static Makefile and a configure.sh seem like very bad ideas to me. Why do 
> > you need those over cmake?
> 
> Mathieu Tarral wrote:
> > Why is this per repo?
> 
> Because i didn't know how to implement it in another way.
> Another idea is to implement it as a plugin in the krazy framework,
> so you can run it over any KDE repo without having them to implement it 
> in their code.
> 
> > Why do you need those over cmake?
> 
> For new developers, it avoids them to learn some CMake specific variables 
> like CMAKE_INSTALL_PREFIX and CMAKE_CXX_COMPILER.
> And for developers in general, it allows them to configure the project 
> really quickly.
> It's like a very little abstraction to the CMake interface.
> 
> Milian Wolff wrote:
> The makefile and configure script are a really bad idea. Maybe you think 
> its simpler for newcomers, I find it highly confusing. You need to know CMake 
> in KDE, so use CMake everywhere. Furthermore, this will otherwise not work on 
> Windows etc.
> 
> Generally, I think such a feature should probably go into CMake itself, 
> or be put into our ECM if possible. The different analysers (note: there are 
> multiple, just one 'analysis' target is not enough) should be chosen by 
> setting CMAKE_BUILD_TYPE to something like TSAN or ASAN or ...
> 
> Milian Wolff wrote:
> Oh and FTR: afaik one can also do something like this already:
> 
> mkdir build-asan
> cd build-asan
> CXX=clang++ -fsanitize=address cmake ..
> make
> 
> But maybe it won't pass the proper linker flags then. But I'm sure there 
> are ways to do that as well.

You should aim as exposing it as buildtype or similar like milian says, one 
target that sets the proper CXX and linker flags for you.


- Albert


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


On ago. 20, 2014, 10:11 p.m., Mathieu Tarral wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119875/
> ---
> 
> (Updated ago. 20, 2014, 10:11 p.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> This patch add the support of the Clang Static Analyzer 
> (http://clang-analyzer.llvm.org/) implemented using a CMake target.
> For each project in the main tree, it creates a 
> _static_analysis target which sets the c++-analyzer compiler
> and then call scan-build. (see the cmake module for details)
> 
> As I couldn't find how to change the compiler for a specific target in CMake, 
> I had to create another a CMake sub-build tree
> named static_analysis_named in the main CMake build tree, and then 
> reconfigure the project by forcing
> the C++ compiler to c++-analyzer.
> 
> Also I added a configure.sh script that allow to set the build options very 
> easily, using dialog.
> So, new developers can change the compiler or set the install prefix without 
> having to know
> the specific CMake variables.
> 
> And finally i added a root Makefile, so you don't have to stay in the build 
> directory to
> compile your targets.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt b06ba01fff7cee445fcc0b896cef041bd4c34fc8 
>   Makefile PRE-CREATION 
>   cmake/modules/clang_static_analysis.cmake PRE-CREATION 
>   configure.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119875/diff/
> 
> 
> Testing
> ---
> 
> For configure.sh :
> ./configure.sh
> ./configure.sh -c
> ./configure.sh -b
> ./configuer.sh -p
> 
> for clang static analyzer
> ./configure.sh
> make dolphin_static_analysis
> 
> 
> Thanks,
> 
> Mathieu Tarral
> 
>

Re: Review Request 119868: KConfig add DirectoryType support

[Maarten De Meyer]

> On Aug. 21, 2014, 7:19 a.m., David Faure wrote:
> > We have never needed this until now because we can recognize such files by 
> > their name... So while I'm not opposed to this, I wonder, why not use if 
> > (u.fileName() == ".directory") in KDesktopFileActions (RR 119869) -- which 
> > will also make it work with existing .directory files that got created 
> > without a Type=Directory entry?

The simple solution is always the best one.
Thanks! :)


- Maarten


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


On Aug. 21, 2014, 1:15 p.m., Maarten De Meyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119868/
> ---
> 
> (Updated Aug. 21, 2014, 1:15 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and Frank Reininghaus.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Add a hasDirectoryType method to KDesktopFile.
> This is required for review request 
> [119869](https://git.reviewboard.kde.org/r/119869/)
> 
> 
> Diffs
> -
> 
>   src/core/kdesktopfile.h 2190051 
>   src/core/kdesktopfile.cpp 6278309 
> 
> Diff: https://git.reviewboard.kde.org/r/119868/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maarten De Meyer
> 
>

Re: Review Request 119868: KConfig add DirectoryType support

[Maarten De Meyer]

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

(Updated Aug. 21, 2014, 1:15 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks, kdelibs and Frank Reininghaus.


Repository: kconfig


Description
---

Add a hasDirectoryType method to KDesktopFile.
This is required for review request 
[119869](https://git.reviewboard.kde.org/r/119869/)


Diffs
-

  src/core/kdesktopfile.h 2190051 
  src/core/kdesktopfile.cpp 6278309 

Diff: https://git.reviewboard.kde.org/r/119868/diff/


Testing
---


Thanks,

Maarten De Meyer

Re: Review Request 119869: KIO: Correctly handle .directory files

[Maarten De Meyer]

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

(Updated Aug. 21, 2014, 1:15 p.m.)


Review request for KDE Frameworks, kdelibs and Frank Reininghaus.


Bugs: 235457
https://bugs.kde.org/show_bug.cgi?id=235457


Repository: kio


Description
---

My attempt at fixing [bug 235457](https://bugs.kde.org/show_bug.cgi?id=235457)
We give .directory the default Type 'Directory' and a way for 
KDesktopFileActions to handle .directory files.
To do that we trick KRun in opening the file as plain text.

hasDirectoryType is a new method in KConfig. see: 
[119868](https://git.reviewboard.kde.org/r/119868/) 
Anything I should look out for when commiting this to KConfig and KIO?


Diffs (updated)
-

  src/widgets/kdesktopfileactions.cpp 9486015 

Diff: https://git.reviewboard.kde.org/r/119869/diff/


Testing
---

Configure a folder with a custom icon in dolphin. Click the created .directory 
file.


Thanks,

Maarten De Meyer

Re: Review Request 119875: add support for Clang Static Analyzer and improve build configuration

[Milian Wolff]

> On Aug. 20, 2014, 10:15 p.m., Albert Astals Cid wrote:
> > Why is this per repo? This should be in a place all repos get the nice 
> > stuff without having to do work.
> > 
> > A static Makefile and a configure.sh seem like very bad ideas to me. Why do 
> > you need those over cmake?
> 
> Mathieu Tarral wrote:
> > Why is this per repo?
> 
> Because i didn't know how to implement it in another way.
> Another idea is to implement it as a plugin in the krazy framework,
> so you can run it over any KDE repo without having them to implement it 
> in their code.
> 
> > Why do you need those over cmake?
> 
> For new developers, it avoids them to learn some CMake specific variables 
> like CMAKE_INSTALL_PREFIX and CMAKE_CXX_COMPILER.
> And for developers in general, it allows them to configure the project 
> really quickly.
> It's like a very little abstraction to the CMake interface.
> 
> Milian Wolff wrote:
> The makefile and configure script are a really bad idea. Maybe you think 
> its simpler for newcomers, I find it highly confusing. You need to know CMake 
> in KDE, so use CMake everywhere. Furthermore, this will otherwise not work on 
> Windows etc.
> 
> Generally, I think such a feature should probably go into CMake itself, 
> or be put into our ECM if possible. The different analysers (note: there are 
> multiple, just one 'analysis' target is not enough) should be chosen by 
> setting CMAKE_BUILD_TYPE to something like TSAN or ASAN or ...

Oh and FTR: afaik one can also do something like this already:

mkdir build-asan
cd build-asan
CXX=clang++ -fsanitize=address cmake ..
make

But maybe it won't pass the proper linker flags then. But I'm sure there are 
ways to do that as well.


- Milian


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


On Aug. 20, 2014, 10:11 p.m., Mathieu Tarral wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119875/
> ---
> 
> (Updated Aug. 20, 2014, 10:11 p.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> This patch add the support of the Clang Static Analyzer 
> (http://clang-analyzer.llvm.org/) implemented using a CMake target.
> For each project in the main tree, it creates a 
> _static_analysis target which sets the c++-analyzer compiler
> and then call scan-build. (see the cmake module for details)
> 
> As I couldn't find how to change the compiler for a specific target in CMake, 
> I had to create another a CMake sub-build tree
> named static_analysis_named in the main CMake build tree, and then 
> reconfigure the project by forcing
> the C++ compiler to c++-analyzer.
> 
> Also I added a configure.sh script that allow to set the build options very 
> easily, using dialog.
> So, new developers can change the compiler or set the install prefix without 
> having to know
> the specific CMake variables.
> 
> And finally i added a root Makefile, so you don't have to stay in the build 
> directory to
> compile your targets.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt b06ba01fff7cee445fcc0b896cef041bd4c34fc8 
>   Makefile PRE-CREATION 
>   cmake/modules/clang_static_analysis.cmake PRE-CREATION 
>   configure.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119875/diff/
> 
> 
> Testing
> ---
> 
> For configure.sh :
> ./configure.sh
> ./configure.sh -c
> ./configure.sh -b
> ./configuer.sh -p
> 
> for clang static analyzer
> ./configure.sh
> make dolphin_static_analysis
> 
> 
> Thanks,
> 
> Mathieu Tarral
> 
>

Re: Review Request 119875: add support for Clang Static Analyzer and improve build configuration

[Milian Wolff]

> On Aug. 20, 2014, 10:15 p.m., Albert Astals Cid wrote:
> > Why is this per repo? This should be in a place all repos get the nice 
> > stuff without having to do work.
> > 
> > A static Makefile and a configure.sh seem like very bad ideas to me. Why do 
> > you need those over cmake?
> 
> Mathieu Tarral wrote:
> > Why is this per repo?
> 
> Because i didn't know how to implement it in another way.
> Another idea is to implement it as a plugin in the krazy framework,
> so you can run it over any KDE repo without having them to implement it 
> in their code.
> 
> > Why do you need those over cmake?
> 
> For new developers, it avoids them to learn some CMake specific variables 
> like CMAKE_INSTALL_PREFIX and CMAKE_CXX_COMPILER.
> And for developers in general, it allows them to configure the project 
> really quickly.
> It's like a very little abstraction to the CMake interface.

The makefile and configure script are a really bad idea. Maybe you think its 
simpler for newcomers, I find it highly confusing. You need to know CMake in 
KDE, so use CMake everywhere. Furthermore, this will otherwise not work on 
Windows etc.

Generally, I think such a feature should probably go into CMake itself, or be 
put into our ECM if possible. The different analysers (note: there are 
multiple, just one 'analysis' target is not enough) should be chosen by setting 
CMAKE_BUILD_TYPE to something like TSAN or ASAN or ...


- Milian


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


On Aug. 20, 2014, 10:11 p.m., Mathieu Tarral wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119875/
> ---
> 
> (Updated Aug. 20, 2014, 10:11 p.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> This patch add the support of the Clang Static Analyzer 
> (http://clang-analyzer.llvm.org/) implemented using a CMake target.
> For each project in the main tree, it creates a 
> _static_analysis target which sets the c++-analyzer compiler
> and then call scan-build. (see the cmake module for details)
> 
> As I couldn't find how to change the compiler for a specific target in CMake, 
> I had to create another a CMake sub-build tree
> named static_analysis_named in the main CMake build tree, and then 
> reconfigure the project by forcing
> the C++ compiler to c++-analyzer.
> 
> Also I added a configure.sh script that allow to set the build options very 
> easily, using dialog.
> So, new developers can change the compiler or set the install prefix without 
> having to know
> the specific CMake variables.
> 
> And finally i added a root Makefile, so you don't have to stay in the build 
> directory to
> compile your targets.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt b06ba01fff7cee445fcc0b896cef041bd4c34fc8 
>   Makefile PRE-CREATION 
>   cmake/modules/clang_static_analysis.cmake PRE-CREATION 
>   configure.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119875/diff/
> 
> 
> Testing
> ---
> 
> For configure.sh :
> ./configure.sh
> ./configure.sh -c
> ./configure.sh -b
> ./configuer.sh -p
> 
> for clang static analyzer
> ./configure.sh
> make dolphin_static_analysis
> 
> 
> Thanks,
> 
> Mathieu Tarral
> 
>

Re: Review Request 119868: KConfig add DirectoryType support

[David Faure]

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


We have never needed this until now because we can recognize such files by 
their name... So while I'm not opposed to this, I wonder, why not use if 
(u.fileName() == ".directory") in KDesktopFileActions (RR 119869) -- which will 
also make it work with existing .directory files that got created without a 
Type=Directory entry?


src/core/kdesktopfile.h


wheter -> whether
an -> a



src/core/kdesktopfile.h


@since 5.2


- David Faure


On Aug. 20, 2014, 10:34 p.m., Maarten De Meyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119868/
> ---
> 
> (Updated Aug. 20, 2014, 10:34 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and Frank Reininghaus.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Add a hasDirectoryType method to KDesktopFile.
> This is required for review request 
> [119869](https://git.reviewboard.kde.org/r/119869/)
> 
> 
> Diffs
> -
> 
>   src/core/kdesktopfile.h 2190051 
>   src/core/kdesktopfile.cpp 6278309 
> 
> Diff: https://git.reviewboard.kde.org/r/119868/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maarten De Meyer
> 
>