Re: Review Request: DrKonqi parses bactraces to find duplicates

2011-10-23 Thread Matthias Fuchs

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

(Updated Oct. 23, 2011, 1:51 p.m.)


Review request for KDE Runtime and George Kiagiadakis.


Changes
---

Thanks for all the feedback.
This patch implements your suggestions.


Description
---

This patch allows DrKonqi to download the possible duplicate bug reports and 
parse their backtraces.
If a report turns out as duplicate reporting a new bug is not possible anymore.
Thus the number of duplicates reported via DrKonqi should hopefully decline.

Note: The comparing backtraces part is not that good. Unfortunately I do not 
have much time to find a good algorithm. In the worst case we could make sure 
that only perfect duplicates are declined, that still should improve the 
current situation imo.


Diffs (updated)
-

  drkonqi/CMakeLists.txt e0c2f6f 
  drkonqi/bugzillalib.h 1e970a0 
  drkonqi/bugzillalib.cpp aa558a9 
  drkonqi/duplicatefinderjob.h PRE-CREATION 
  drkonqi/duplicatefinderjob.cpp PRE-CREATION 
  drkonqi/parsebugbacktraces.h PRE-CREATION 
  drkonqi/parsebugbacktraces.cpp PRE-CREATION 
  drkonqi/reportassistantdialog.cpp 43731b0 
  drkonqi/reportassistantpages_base.cpp 4a25c45 
  drkonqi/reportassistantpages_bugzilla_duplicates.h b0b4462 
  drkonqi/reportassistantpages_bugzilla_duplicates.cpp 30e861d 
  drkonqi/reportinterface.h c7f645b 
  drkonqi/reportinterface.cpp 90e00b3 
  drkonqi/tests/.CMakeLists.txt~ PRE-CREATION 
  drkonqi/ui/assistantpage_bugzilla_duplicates.ui 1ec034a 

Diff: http://git.reviewboard.kde.org/r/102921/diff/diff


Testing
---


Thanks,

Matthias Fuchs



Review Request: DrKonqi parses bactraces to find duplicates

2011-10-19 Thread Matthias Fuchs

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

Review request for KDE Runtime.


Description
---

This patch allows DrKonqi to download the possible duplicate bug reports and 
parse their backtraces.
If a report turns out as duplicate reporting a new bug is not possible anymore.
Thus the number of duplicates reported via DrKonqi should hopefully decline.

Note: The comparing backtraces part is not that good. Unfortunately I do not 
have much time to find a good algorithm. In the worst case we could make sure 
that only perfect duplicates are declined, that still should improve the 
current situation imo.


Diffs
-

  drkonqi/CMakeLists.txt e0c2f6f 
  drkonqi/bugzillalib.h 1e970a0 
  drkonqi/bugzillalib.cpp aa558a9 
  drkonqi/duplicatefinderjob.h PRE-CREATION 
  drkonqi/duplicatefinderjob.cpp PRE-CREATION 
  drkonqi/parsebugbacktraces.h PRE-CREATION 
  drkonqi/parsebugbacktraces.cpp PRE-CREATION 
  drkonqi/reportassistantdialog.cpp 43731b0 
  drkonqi/reportassistantpages_base.cpp 4a25c45 
  drkonqi/reportassistantpages_bugzilla_duplicates.h b0b4462 
  drkonqi/reportassistantpages_bugzilla_duplicates.cpp 30e861d 
  drkonqi/reportinterface.h c7f645b 
  drkonqi/reportinterface.cpp 90e00b3 
  drkonqi/ui/assistantpage_bugzilla_duplicates.ui 1ec034a 

Diff: http://git.reviewboard.kde.org/r/102921/diff/diff


Testing
---


Thanks,

Matthias Fuchs



Re: Screensaver to be or not to be

2011-10-13 Thread Matthias Fuchs

Am 13.10.2011 21:09, schrieb Martin Gräßlin:

On Thursday 13 October 2011 20:49:29 Matthias Fuchs wrote:

I understand and agree to the technical reasons against keeping the
current architecture but that does not mean that I agree to all the
other arguments.

I just want to point out once more that we will not remove anything which is
possible today. The only thing we remove is the coupling of screen
savers/animation with screen locker.

Yes I understood that and do not argue on that.

I simply oppose the way some people argue on screensavers in general.



Re: Screensaver to be or not to be

2011-10-13 Thread Matthias Fuchs

Am 13.10.2011 20:21, schrieb Thomas Lübking:

[...] it now had the chance to save a lot of energy AND the

screen by simply turning the screen off, since nobody is watching.
> [...]

So the only sane reason these things to show up "automatically" is that
the system is abandoned and than it's no more sane to run them.


That assumption is wrong.
First of all why should there be different screen savers if none was 
looking at them?


Second I just saw a screensaver the last days -- not my machine -- how 
is that possible if nobody is watching? Well if you are in an office 
there are more PCs than just yours.
Something like a screen saver is for sure not bad for motivation or 
mood, rather the opposite.


Further being in front of the pc does not mean that you are constantly 
interacting with it. Sometimes you are noting things, reading some 
information which is on paper etc. That is the moment something like a 
"screen-prettisers" (better than saver? ;) ) kicks in which might cause 
a little cheering up.



The argument of saving energy is kinda hypocritical if you look at what 
most people use their computer for. If energy saving is the reason to 
deactivate things that some people enjoy -- they actually see these 
things from time to time -- then anything flash related should be 
deactivated, youtube should not be watched and even better the pc should 
stay turned off most of the time for most people.


Thus energy saving is imo not a valid argument here. Screen prettisers 
are off by default and by default none forces people to visit youtube.

If they decide so, well fine for them.


I understand and agree to the technical reasons against keeping the 
current architecture but that does not mean that I agree to all the 
other arguments.


Review Request: DrKonqi uses a more flexible way to retrieve the version

2011-08-31 Thread Matthias Fuchs

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

Review request for KDE Runtime and Darío Andrés Rodríguez.


Summary
---

Instead of relying on the bug reporting page this patch uses config.cgi to 
retrieve the version of a specified product.


Diffs
-

  drkonqi/CMakeLists.txt 10c1846 
  drkonqi/bugzillalib.h 1c7e895 
  drkonqi/bugzillalib.cpp 7664650 
  drkonqi/findconfigdatajob.h PRE-CREATION 
  drkonqi/findconfigdatajob.cpp PRE-CREATION 
  drkonqi/productmapping.cpp a58e240 
  drkonqi/tests/bugzillalibtest/CMakeLists.txt c85f8fd 

Diff: http://git.reviewboard.kde.org/r/102497/diff


Testing
---


Thanks,

Matthias



Review Request: Inserts the program version in more cases in bug reports.

2011-08-28 Thread Matthias Fuchs

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

Review request for KDE Runtime and Darío Andrés Rodríguez.


Summary
---

As it turns out the release of KDE 4.7.0 is called "4.7.00" so with two 0s, as 
was also true for 4.6.0.
Yet on bugs.kde.org there often is just e.g. version "4.7.0" available, thus 
DrKonqi does not insert the version in the version field.
Now DrKonqi treats "4.7.0" and "4.7.00" as equal when determaining if the 
verion should be entered in the version field.


Diffs
-

  drkonqi/productmapping.cpp efb3b11 

Diff: http://git.reviewboard.kde.org/r/102479/diff


Testing
---


Thanks,

Matthias



Re: -Wunused-but-set-variable warnings

2011-07-04 Thread Matthias Fuchs
Am Montag 04 Juli 2011, 20:07:20 schrieb Albert Astals Cid:
> A Monday, July 04, 2011, Dawit A va escriure:
> > The following files all contain set but unused variables:
> > 
> > snip
> > 
> > Unlike the -Wunused-parameter fixing this warning messages requires
> > context because the variable may be set and unused due to a mistake
> > that can potentially be causing a bug. As such can kdelibs cmake file
> > be changed to error out, -Werror=unused-but-set-variable, for such
> > warnings ?
> 
> You really want to make kdelibs uncompilable?
> 
> Albert

Indeed. IIRC in some cases the public API has unused members. Removing them 
could lead to ABI problems.


Review Request: Return the url of the view instead of the url of the url navigator.

2011-06-10 Thread Matthias Fuchs

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

Review request for KDE Base Apps and Peter Penz.


Summary
---

That way if a wrong protocol had been entered the currently watched directory 
will be returned.


This addresses bug 274890.
http://bugs.kde.org/show_bug.cgi?id=274890


Diffs
-

  dolphin/src/dolphinviewcontainer.cpp 1042ece 

Diff: http://git.reviewboard.kde.org/r/101580/diff


Testing
---


Thanks,

Matthias



Review Request: Show icon overlays in the Informationen Panel.

2011-06-10 Thread Matthias Fuchs

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

Review request for KDE Base Apps and Peter Penz.


Summary
---

Depends on https://git.reviewboard.kde.org/r/101569/


This addresses bug 190579.
http://bugs.kde.org/show_bug.cgi?id=190579


Diffs
-

  dolphin/src/panels/information/informationpanelcontent.cpp 77a6232 

Diff: http://git.reviewboard.kde.org/r/101573/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Draw overlays even for previews.

2011-06-10 Thread Matthias Fuchs

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

(Updated June 10, 2011, 7:46 p.m.)


Review request for kdelibs, David Faure and Peter Penz.


Summary (updated)
---

This way it is easier to recognise links to images etc.
Depends on https://git.reviewboard.kde.org/r/101569/


This addresses bug 190579.
http://bugs.kde.org/show_bug.cgi?id=190579


Diffs
-

  kfile/kfilepreviewgenerator.cpp 216dd7e 

Diff: http://git.reviewboard.kde.org/r/101570/diff


Testing
---


Screenshots
---


  http://git.reviewboard.kde.org/r/101570/s/179/


Thanks,

Matthias



Review Request: Draw overlays even for previews.

2011-06-10 Thread Matthias Fuchs

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

Review request for kdelibs, David Faure and Peter Penz.


Summary
---

This way it is easier to recognise links to images etc.


This addresses bug 190579.
http://bugs.kde.org/show_bug.cgi?id=190579


Diffs
-

  kfile/kfilepreviewgenerator.cpp 216dd7e 

Diff: http://git.reviewboard.kde.org/r/101570/diff


Testing
---


Screenshots
---


  http://git.reviewboard.kde.org/r/101570/s/179/


Thanks,

Matthias



Review Request: Adds KIconLoader::drawOverlays which allows to draw overlays on any pixmap.

2011-06-10 Thread Matthias Fuchs

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

Review request for kdelibs, David Faure and Michael Pyne.


Summary
---

This method uses KIconLoaderPrivate::drawOverlays which is also modified by 
this patch, to take both width and height of the pixmaps into consideration.

I plan to utilize this method both in the Dolphin Information Panel and in 
KFilePreviewGenerator.


Diffs
-

  kdeui/icons/kiconloader.h c29c12e 
  kdeui/icons/kiconloader.cpp bcb7b69 

Diff: http://git.reviewboard.kde.org/r/101569/diff


Testing
---


Thanks,

Matthias



Creating a public pixmap overlays method based on icon overlays?

2011-06-02 Thread Matthias Fuchs
Hi,

I was looking at BUG:190579 [1] and realised that there is a general problem 
in KDE that does not only apply to folders:
As soon as you have preview enabled you won't get any more icon overlays for 
images, no matter if you look at images directly or at folders containing 
images (the later most likely because the previews are pointed over the 
overlay).

So I thought what should be done is to also display overlays for previews. Yet 
the overlay method only applies to icons at the moment. [2]

The code that is used [3] can be easily adpated to pixmaps not having the same 
width and height. [4]
I even tried it and it looks good enough in my opinion.

Now I need your feedback:
Should a more general method like that be added?
Should that method be used for previews and also KIconLoader?
And where should such a method be placed, I mean in which header?


Cheers,
matthias

[1] https://bugs.kde.org/show_bug.cgi?id=190579
[2] KIcon (const QString &iconName, KIconLoader *iconLoader, const QStringList 
&overlays) and KIconLoader::loadIcon
[3] KIconLoaderPrivate::drawOverlays at kiconloader.cpp:357
[4] something like:
const int height = pix.size().height();
const int width = pix.size().width();
const int iconSize = qMin(width, height);
...
and then using height/width instead of iconSize for the x/y values of the 
following QPoints.


Re: Review Request: Keeps the selection after showing/hiding hidden files.

2011-06-01 Thread Matthias Fuchs


> On June 1, 2011, 10:48 p.m., Peter Penz wrote:
> > Thanks for the patch. I just had a look and if I don't miss anything it 
> > should
> > be sufficient to just add the line:
> >   m_selectedItems = selectedItems();
> > to DolphinView::setShowHiddenFiles().
> > 
> > I'm not really happy with how the selections are remembered in DolphinView 
> > in
> > general: We have m_selectedItems to remember the selection when e.g. 
> > changing
> > views and there is m_newFileNames for the usecase where the KFileItems are 
> > not
> > available yet and only the new names are known.
> > 
> > I'm currently working on a (let's say) "new view-engine" in Dolphin for 4.8
> > that should allow us to get rid of at least m_selectedItems but for 4.7 it
> > would be great if we could get in this fix.
> > 
> > Would it be possible that you check whether 'm_selectedItems =
> > selectedItems();' is sufficient? I'm quite sure this should work but 
> > probably
> > I'm missing something... Thanks :-)

Indeed that works too and is a lot (!) nicer. :)

Yeah the m_newFileNames is not really nice and can't handle multiple corner 
cases like kio operations that were paused in between e.g. because of existing 
files etc.


- Matthias


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


On June 1, 2011, 9:58 p.m., Matthias Fuchs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101487/
> ---
> 
> (Updated June 1, 2011, 9:58 p.m.)
> 
> 
> Review request for KDE Base Apps, Peter Penz and Frank Reininghaus.
> 
> 
> Summary
> ---
> 
> Still only files that are shown will be selected, thus selecting hidden files 
> and then hiding hidden files will deselect those.
> 
> 
> This addresses bug 177215.
> http://bugs.kde.org/show_bug.cgi?id=177215
> 
> 
> Diffs
> -
> 
>   dolphin/src/views/dolphinview.cpp 4bc901b 
> 
> Diff: http://git.reviewboard.kde.org/r/101487/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthias
> 
>



Review Request: Keeps the selection after showing/hiding hidden files.

2011-06-01 Thread Matthias Fuchs

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

Review request for KDE Base Apps, Peter Penz and Frank Reininghaus.


Summary
---

Still only files that are shown will be selected, thus selecting hidden files 
and then hiding hidden files will deselect those.


This addresses bug 177215.
http://bugs.kde.org/show_bug.cgi?id=177215


Diffs
-

  dolphin/src/views/dolphinview.cpp 4bc901b 

Diff: http://git.reviewboard.kde.org/r/101487/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Disallow names of files containing '/' or being equal to "." or "..".

2011-05-29 Thread Matthias Fuchs


> On May 28, 2011, 9:29 a.m., David Faure wrote:
> > The handling of '/' could use KIO::encodeFileName instead, for consistency. 
> > I agree about "." and ".." though ;)
> 
> Mark Gaiser wrote:
> I wonder.. don't we need a "isFilename" function that checks if a 
> filename meets all the characters that are allowed in a filename thus 
> returning false when it starts with a / or for example contains a *? or being 
> equal to . or .. ? That would make the use more "portable" since i can 
> imagine other apps benefiting from that (any app that needs to create a file 
> or folder from user input)
> 
> Richard J. Moore wrote:
> Filenames containing * and ? are perfectly legal. The only truly illegal 
> characters in a unix filename are / and \0. To be portable, you need to 
> consider the underlying filesystem in which a file is being created since 
> this sets the rules, for example FAT has a bunch of characters that can't be 
> used (eg. consider how a filename starting with ? would interact with its 
> file deletion handling). Short version, I don't see how the function you 
> propose could be implemented.
>
> 
> Mark Gaiser wrote:
> You are completely right. I didn't think about the differences per 
> filesystem.

Well if a function like that was to be done it would need at least two 
parameters.
1. tried file name
2. (target)location of the file

Then you could easily find the mount point and the file system via:
KMountPoint::Ptr mountPoint = 
KMountPoint::currentMountPoints().findByPath(fileUrl.directory());
const QString mountType = mountPoint->mountType();

Now what I fear is that this could be a costly operation to do that for many 
files, e.g. when you do a copy operation.
And the result could only be cached for the same directory, yet not for similar 
ones as you could mount a different file system in a sub directory.


- Matthias


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


On May 28, 2011, 6:26 p.m., Matthias Fuchs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101456/
> ---
> 
> (Updated May 28, 2011, 6:26 p.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> ---
> 
> nt
> 
> 
> This addresses bug 211751.
> http://bugs.kde.org/show_bug.cgi?id=211751
> 
> 
> Diffs
> -
> 
>   kio/kio/kdirmodel.cpp 6bf57be 
>   kio/kio/kfileitemdelegate.cpp cb3939d 
> 
> Diff: http://git.reviewboard.kde.org/r/101456/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthias
> 
>



Re: Review Request: Disallow names of files containing '/' or being equal to "." or "..".

2011-05-28 Thread Matthias Fuchs

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

(Updated May 28, 2011, 6:26 p.m.)


Review request for kdelibs and David Faure.


Changes
---

Uses KIO::encodeFileName.

@David could KIO::encodeFileName lead to problems if file systems are used that 
do not use UTF8?


Summary
---

nt


This addresses bug 211751.
http://bugs.kde.org/show_bug.cgi?id=211751


Diffs (updated)
-

  kio/kio/kdirmodel.cpp 6bf57be 
  kio/kio/kfileitemdelegate.cpp cb3939d 

Diff: http://git.reviewboard.kde.org/r/101456/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Disallows renaming to names containing '/' names being equal to "." or "..".

2011-05-27 Thread Matthias Fuchs


> On May 27, 2011, 11:33 p.m., Commit Hook wrote:
> > This review has been submitted with commit 
> > 8dbc4b0752dc12121c37000c764a7e025daae4b7 by Matthias Fuchs.

The review request for the changes in KDirModel and KFileItemDelegate can be 
found here:
https://git.reviewboard.kde.org/r/101456/


- Matthias


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


On May 27, 2011, 11:07 p.m., Matthias Fuchs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101454/
> ---
> 
> (Updated May 27, 2011, 11:07 p.m.)
> 
> 
> Review request for KDE Base Apps and Peter Penz.
> 
> 
> Summary
> ---
> 
> So far this works only for the RenameDialog as I could not find the method 
> that changes the name when inline renaming is enabled.
> If you could tell me where that method is I would in fact adapt the patch.
> 
> 
> This addresses bug 211751.
> http://bugs.kde.org/show_bug.cgi?id=211751
> 
> 
> Diffs
> -
> 
>   dolphin/src/views/renamedialog.cpp 810562a 
> 
> Diff: http://git.reviewboard.kde.org/r/101454/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthias
> 
>



Review Request: Disallow names of files containing '/' or being equal to "." or "..".

2011-05-27 Thread Matthias Fuchs

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

Review request for kdelibs and David Faure.


Summary
---

nt


This addresses bug 211751.
http://bugs.kde.org/show_bug.cgi?id=211751


Diffs
-

  kio/kio/kdirmodel.cpp 6bf57be 
  kio/kio/kfileitemdelegate.cpp cb3939d 

Diff: http://git.reviewboard.kde.org/r/101456/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Disallows renaming to names containing '/' names being equal to "." or "..".

2011-05-27 Thread Matthias Fuchs

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

(Updated May 27, 2011, 11:07 p.m.)


Review request for KDE Base Apps and Peter Penz.


Changes
---

forgot title


Summary (updated)
---

So far this works only for the RenameDialog as I could not find the method that 
changes the name when inline renaming is enabled.
If you could tell me where that method is I would in fact adapt the patch.


This addresses bug 211751.
http://bugs.kde.org/show_bug.cgi?id=211751


Diffs
-

  dolphin/src/views/renamedialog.cpp 810562a 

Diff: http://git.reviewboard.kde.org/r/101454/diff


Testing
---


Thanks,

Matthias



Review Request: Jar archives also uses the zip protocol.

2011-05-27 Thread Matthias Fuchs

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

Review request for KDE Runtime and David Faure.


Summary
---

nt


This addresses bug 188989.
http://bugs.kde.org/show_bug.cgi?id=188989


Diffs
-

  kioslave/archive/zip.protocol ce7c54b 

Diff: http://git.reviewboard.kde.org/r/101455/diff


Testing
---


Thanks,

Matthias



Review Request:

2011-05-27 Thread Matthias Fuchs

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

Review request for KDE Base Apps and Peter Penz.


Summary
---

So far this works only for the RenameDialog as I could not find the method that 
changes the name when inline renaming is enabled.
If you could tell me where that method is I would in fact adapt the patch.


This addresses bug 211751.
http://bugs.kde.org/show_bug.cgi?id=211751


Diffs
-

  dolphin/src/views/renamedialog.cpp 810562a 

Diff: http://git.reviewboard.kde.org/r/101454/diff


Testing
---


Thanks,

Matthias



Review Request: DolphinColumnView navigation works more intuitively.

2011-05-27 Thread Matthias Fuchs

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

Review request for KDE Base Apps and Peter Penz.


Summary
---

If no item is selected then pressing right moves to a column view with child 
url, instead of the first index.


This addresses bug 263110.
http://bugs.kde.org/show_bug.cgi?id=263110


Diffs
-

  dolphin/src/views/dolphincolumnview.h be50ea3 
  dolphin/src/views/dolphincolumnview.cpp fa1f620 

Diff: http://git.reviewboard.kde.org/r/101449/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Ignores dots at the beginning of a file name when suggesting a file name

2011-05-24 Thread Matthias Fuchs


> On May 24, 2011, 6:08 p.m., Commit Hook wrote:
> > This review has been submitted with commit 
> > ac6790ebf207c566b14a9041974d60df7741d415 by Matthias Fuchs.

Also commited it to master f2a6626f9fc1a5ed07d7115ef79d05273d8522b7 


- Matthias


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


On May 24, 2011, 10:55 a.m., Matthias Fuchs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101429/
> ---
> 
> (Updated May 24, 2011, 10:55 a.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> ---
> 
> This ensures that ".aFile.tar.gz" becomes ".aFile 1.tar.gz" instead of " 
> 1.aFile.tar.gz".
> 
> I would also backport this to the 4.6 branch.
> 
> 
> Diffs
> -
> 
>   kio/kio/renamedialog.cpp 9afb872 
> 
> Diff: http://git.reviewboard.kde.org/r/101429/diff
> 
> 
> Testing
> ---
> 
> Test with files with the following names "a", ".a", "aFile.tar", 
> "aFile.tar.gz", ".aFile.tar.gz". Works as described.
> 
> 
> Thanks,
> 
> Matthias
> 
>



Re: Review Request: Display the tab title on root (/) folder properly in konqueror filemanager mode

2011-05-24 Thread Matthias Fuchs


> On May 24, 2011, 5:59 p.m., Commit Hook wrote:
> > This review has been submitted with commit 
> > f2a6626f9fc1a5ed07d7115ef79d05273d8522b7 by Matthias Fuchs.

Sorry copied this review id by mistake, my commit is for 101429 so ignore it.


- Matthias


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


On May 23, 2011, 8:31 p.m., Burkhard Lück wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101374/
> ---
> 
> (Updated May 23, 2011, 8:31 p.m.)
> 
> 
> Review request for KDE Base Apps, David Faure and Peter Penz.
> 
> 
> Summary
> ---
> 
> Konqueror in filemanager mode shows an empty tab title on browsing root (/) 
> folder. Dolphin displays the tab title on root (/) folder properly, so this 
> patch is a copy of three lines from dolphin dolphinmainwindow.cpp.
> 
> 
> This addresses bug 153573.
> http://bugs.kde.org/show_bug.cgi?id=153573
> 
> 
> Diffs
> -
> 
>   konqueror/src/konqview.cpp 699c9d5 
> 
> Diff: http://git.reviewboard.kde.org/r/101374/diff
> 
> 
> Testing
> ---
> 
> compiled and works for me
> 
> 
> Thanks,
> 
> Burkhard
> 
>



Re: Review Request: Ignores dots at the beginning of a file name when suggesting a file name

2011-05-24 Thread Matthias Fuchs

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

(Updated May 24, 2011, 10:55 a.m.)


Review request for kdelibs and David Faure.


Changes
---

Now handles any number of preceding dots ('.').
Tested with files called "...", "..aFile.tar.gz" as well as the previous 
mentioned ones.

Just in case: Are there unit tests for the suggestNewName method that I could 
run as well?


Summary
---

This ensures that ".aFile.tar.gz" becomes ".aFile 1.tar.gz" instead of " 
1.aFile.tar.gz".

I would also backport this to the 4.6 branch.


Diffs (updated)
-

  kio/kio/renamedialog.cpp 9afb872 

Diff: http://git.reviewboard.kde.org/r/101429/diff


Testing
---

Test with files with the following names "a", ".a", "aFile.tar", 
"aFile.tar.gz", ".aFile.tar.gz". Works as described.


Thanks,

Matthias



Re: Review Request: Ignores dots at the beginning of a file name when suggesting a file name

2011-05-23 Thread Matthias Fuchs

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

(Updated May 23, 2011, 9:02 p.m.)


Review request for kdelibs and David Faure.


Summary (updated)
---

This ensures that ".aFile.tar.gz" becomes ".aFile 1.tar.gz" instead of " 
1.aFile.tar.gz".

I would also backport this to the 4.6 branch.


Diffs
-

  kio/kio/renamedialog.cpp 9afb872 

Diff: http://git.reviewboard.kde.org/r/101429/diff


Testing
---

Test with files with the following names "a", ".a", "aFile.tar", 
"aFile.tar.gz", ".aFile.tar.gz". Works as described.


Thanks,

Matthias



Review Request: Ignores dots at the beginning of a file name when suggesting a file name

2011-05-23 Thread Matthias Fuchs

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

Review request for kdelibs and David Faure.


Summary
---

This ensures that ".aFile.tar.gz" becomes ".aFile 1.tar.gz" instead of " 
1.aFile.tar.gz".


Diffs
-

  kio/kio/renamedialog.cpp 9afb872 

Diff: http://git.reviewboard.kde.org/r/101429/diff


Testing
---

Test with files with the following names "a", ".a", "aFile.tar", 
"aFile.tar.gz", ".aFile.tar.gz". Works as described.


Thanks,

Matthias



Re: kdewebkit compilation fails for Mobile profile in trunk

2011-05-22 Thread Matthias Fuchs
Am Sonntag 22 Mai 2011, 00:26:58 schrieb Dawit A:
> On Sat, May 21, 2011 at 5:50 PM, Andreas Pakulat  wrote:
> > On 21.05.11 13:25:32, Dawit A wrote:
> >> No that is a moc issue. The code is properly @ifdef'ed out in the
> >> code, but the generated moc file does not seem to exclude it. That is
> >> why you get the message:
> >> 
> >> /home/arysin/kde/build/KDE/kdelibs/kdewebkit/kgraphicswebview.moc:91:6:
> >> error: prototype for ‘void
> >> KGraphicsWebView::selectionClipboardUrlPasted(const KUrl&)’ does not
> >> match any in class ‘KGraphicsWebView’
> >> /home/arysin/work/OSS/KDE/kdelibs/kdewebkit/kgraphicswebview.h:146:10:
> >> error: candidate is: void
> >> KGraphicsWebView::selectionClipboardUrlPasted(const KUrl&, const
> >> QString&)
> >> 
> >> Dunno why that would be... Perhaps whomever it is that added the
> >> ifndef around that signal could comment.
> > 
> > Maybe that changed these days, but moc does not know anything about
> > preprocessor-defines - except for a few hardcoded ones. So if you want
> > moc to ignore some code, you'll possibly have to find another way.
> 
> That is why I said whomever added the #ifdef around the signal can
> deal with it. I surely did not put that there since I no need to build
> kdelibs that way. Perhaps there is a trick to work around this
> limitation, since this cannot be the only signal that is deprecated,
> but I do not know what that might be...

Well in that case I think it would be good to contact the right person, and 
that is Kevin Ottens here, so I cced him in that mail.


Re: Review Request: KMessageWidget: Adapt height to text changes

2011-05-08 Thread Matthias Fuchs


> On May 2, 2011, 5:49 p.m., Aurélien Gâteau wrote:
> > Thanks for the patch Mattias! This is indeed an annoying bug but your fix 
> > does not work when the widget is resized. heightForWidth() needs to be 
> > implemented I think.

The problem is that resizing takes no effect yet either. Here without the patch 
enabling word wrap instantly increases the height, even if there is enough 
horizontal space. So the fix would not make matters worse.

I am not sure how to fix the height problem in general though. To me it appears 
that QLabel is lacking here.


- Matthias


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


On May 2, 2011, 12:35 p.m., Matthias Fuchs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101272/
> ---
> 
> (Updated May 2, 2011, 12:35 p.m.)
> 
> 
> Review request for kdelibs and Aurélien Gâteau.
> 
> 
> Summary
> ---
> 
> When there is a long or wrapping text, the text is cut off. This patch fixes 
> that.
> 
> 
> Diffs
> -
> 
>   kdeui/widgets/kmessagewidget.cpp 212ea0d 
> 
> Diff: http://git.reviewboard.kde.org/r/101272/diff
> 
> 
> Testing
> ---
> 
> 
> Screenshots
> ---
> 
> 
>   http://git.reviewboard.kde.org/r/101272/s/154/
> 
>   http://git.reviewboard.kde.org/r/101272/s/155/
> 
> 
> Thanks,
> 
> Matthias
> 
>



Review Request: KMessageWidget: Adapt height to text changes

2011-05-02 Thread Matthias Fuchs

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

Review request for kdelibs and Aurélien Gâteau.


Summary
---

When there is a long or wrapping text, the text is cut off. This patch fixes 
that.


Diffs
-

  kdeui/widgets/kmessagewidget.cpp 212ea0d 

Diff: http://git.reviewboard.kde.org/r/101272/diff


Testing
---


Screenshots
---


  http://git.reviewboard.kde.org/r/101272/s/154/

  http://git.reviewboard.kde.org/r/101272/s/155/


Thanks,

Matthias



Review Request: KNS overwrites existing files automatically on an update

2011-03-27 Thread Matthias Fuchs

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

Review request for kdelibs and Frederik Gladhorn.


Summary
---

The existing code already suggests that the user should not be asked for 
overwriting files if they do an update.
This patch makes this finally work now by looking if the status is 
Entry::Updating.


Diffs
-

  knewstuff/knewstuff3/core/installation.cpp be4ba69 

Diff: http://git.reviewboard.kde.org/r/100961/diff


Testing
---


Thanks,

Matthias



Review Request: Shares the KNS3::Cache amongst users of the same program instance

2011-03-27 Thread Matthias Fuchs

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

Review request for kdelibs and Frederik Gladhorn.


Summary
---

Thus insuring that the cache is always consistent.
This way it can't happen, that e.g. DownloadManager updates some KNS3::Entries 
and a call to DownloadWidget later on would overwrite the updated ones as 
updateable.


Diffs
-

  knewstuff/knewstuff3/core/cache.h 5b0b60e 
  knewstuff/knewstuff3/core/cache.cpp 53aa55e 
  knewstuff/knewstuff3/core/engine.h 5aa7449 
  knewstuff/knewstuff3/core/engine.cpp 1017628 

Diff: http://git.reviewboard.kde.org/r/100960/diff


Testing
---

Tested with the patch in https://git.reviewboard.kde.org/r/100958/ since that 
fixes updateing.

Test case:
Implemented auto-updating for the comic plamsoid.
After some updates happen the user clicks the get more button to install an 
additional comic. In the old behaviour the comic would show up as not installed 
after restarting, as the cache was not shared [1]. With the new behaviour any 
updated comic will show up as installed and the newly installed comic will also 
show up as installed.

[1] Here it only depended which cache was written last to the disk. So either 
all comics would still show up as updatedable or the one comic would show up as 
not installed.


Thanks,

Matthias



Re: Review Request: KNS3 correctly stores updated entries as installed.

2011-03-27 Thread Matthias Fuchs

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

(Updated March 27, 2011, 3:50 p.m.)


Review request for kdelibs and Frederik Gladhorn.


Summary
---

* AtticaProvider also changes the cachedEntry in (!) the cache and it does that 
for Installed and for Updateable entries.
The later is important, since sometimes updates are not done.
* Installation only stores updateReleaseDate if it is valid.


This addresses bug 256473.
http://bugs.kde.org/show_bug.cgi?id=256473


Diffs
-

  knewstuff/knewstuff3/attica/atticaprovider.cpp 33e47ce 
  knewstuff/knewstuff3/core/installation.cpp be4ba69 

Diff: http://git.reviewboard.kde.org/r/100958/diff


Testing
---

Tested with the comic plasmoid.


Thanks,

Matthias



Re: Automount security concerns?

2011-03-11 Thread Matthias Fuchs
Am Freitag 11 März 2011, 19:05:44 schrieb Shaun Reich:
> > Yes physical access is always bad. But imagine you are at a place where
> > many people are (and stealing the pc is no option). Just going to the
> > toilet for a short moment -- with the screen locked -- could make your
> > computer cracked.
> > 
> > 
> > In general I think that nothing usb-stick/new hardware related should
> > happen if the screen is locked.
> 
> I'm assuming said toilet-user has also disabled the (far more
> important) firewire port? ;-)

Not every pc/laptop has one. And btw. just because one part is broken another 
should not be made more secure? With that argueing we could ignore nealy every 
security issue or probable issue. ;)

> 
> >if the screen is locked. And if really a usb-stick is connected to the pc
> >while locked, when a dialog should pop up -- which can only be accessed
> >when unlocking -- asking for further actions.
> 
> No point in that. Simply don't allow mounting when new devices appear
> and the screen is locked. When it is unlocked (it could probably just
> check for d-bus changes, to determine that) and there was a stick
> plugged in which still exists, mount it.

A user choosing to auto mount probably does not see the implications this 
could (!) have on security.
 
> I see your thinking in "show a dialog after an unlock, to let him know
> about it". But there is simply no point in that. If the (toilet-)user
> has physical access to it after finishing his business, has the
> passkey to unlock it, and does so without checking the computer for
> suspicious looking satellite dishes protruding from the PC, then that
> is his prerogative at that point.

USB as means to distribute malware should not be disregarded easily. There was 
a case locally where hundreds of computers were unuseable for days because of 
a virus spread unknowingly via usb.

Showing a dialog would at least help realising that someone else played with 
your machine or that you just tried to connect something to a locked computer.


Yes I don't know how severe this could be in the KDE case, yet of the two 
comments none so far shed more light into this imo, other than automounting is 
turned off by default.


Re: Automount security concerns?

2011-03-11 Thread Matthias Fuchs
In fact I have used plasma ...
Might be that I changed the default and can't remember anymore.

In this case as you pointed out this is most likely a non-issue KDE wise, 
unless the user changed it to auto mounting.

Am Freitag 11 März 2011, 20:27:20 schrieb Markus Slopianka:
> I'm wondering if you took the time to actually try Plasma Desktop before
> posting that mail.
> By default no drive is mounted automatically. Device Notifier just notifies
> that a new drive is present. Users have to click first (either in the
> Plasma popup or Dolphin's side bar) to mount the drive. To get automatic
> mounting, the user has to change settings first. Because of this the gives
> attack case is of no concern.
> 
> Am Freitag 11 März 2011, 18:35:45 schrieb Matthias Fuchs:
> > Hi,
> > 
> > I just watched a video [1] on exploiting autrun/generating of
> > thumbnails/... of data on usb sticks.
> > Yes this is specific to Gnome, though I wonder if that could be a problem
> > in KDE too, as is mentioned at the ending.
> > E.g. I don't know if strigi starts indexing files automatically on
> > mounted stuff.
> > 
> > Yes physical access is always bad. But imagine you are at a place where
> > many people are (and stealing the pc is no option). Just going to the
> > toilet for a short moment -- with the screen locked -- could make your
> > computer cracked.
> > 
> > In general I think that nothing usb-stick/new hardware related should
> > happen if the screen is locked. And if really a usb-stick is connected to
> > the pc while locked, when a dialog should pop up -- which can only be
> > accessed when unlocking -- asking for further actions.
> > This way the risk is reduced and the user gets informed at the same time.
> > 
> > Now where should this happen? Probably in solid, so that nothing being in
> > general informed of new devices will be activated.
> > 
> > [1] http://www.youtube.com/watch?v=ovfYBa1EHm4


Automount security concerns?

2011-03-11 Thread Matthias Fuchs
Hi,

I just watched a video [1] on exploiting autrun/generating of thumbnails/... 
of data on usb sticks.
Yes this is specific to Gnome, though I wonder if that could be a problem in 
KDE too, as is mentioned at the ending.
E.g. I don't know if strigi starts indexing files automatically on mounted 
stuff.

Yes physical access is always bad. But imagine you are at a place where many 
people are (and stealing the pc is no option). Just going to the toilet for a 
short moment -- with the screen locked -- could make your computer cracked.

In general I think that nothing usb-stick/new hardware related should happen 
if the screen is locked. And if really a usb-stick is connected to the pc 
while locked, when a dialog should pop up -- which can only be accessed when 
unlocking -- asking for further actions.
This way the risk is reduced and the user gets informed at the same time.

Now where should this happen? Probably in solid, so that nothing being in 
general informed of new devices will be activated.

[1] http://www.youtube.com/watch?v=ovfYBa1EHm4


Re: Review Request: Implement a dialog to show unhandled Bugzilla errors

2011-02-21 Thread Matthias Fuchs

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



drkonqi/reportassistantpages_bugzilla.cpp


You have to check if the pointer is still valid there.

The reason for using QPointer or QWeakPointer in the first place is that 
the dialog could be destroyed by e.g. a dbus command IIRC --> there was a blog 
entry on how to crash nearly every KDE app which commented on that.

So it could be that it is not valid once it reaches the KUrl fileUrl = ... 
line.
The only way to check if it is valid is using QWeakPointer or QPointer and 
do KUrl fileUrl (dlg ? dlg->selectedUrl() : KUrl());

Yes I suppose in the worst case it could be deleted inbetween but that 
should not happen that often. ;)


- Matthias


On Feb. 19, 2011, 12:33 p.m., Darío Andrés Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100681/
> ---
> 
> (Updated Feb. 19, 2011, 12:33 p.m.)
> 
> 
> Review request for KDE Runtime and George Kiagiadakis.
> 
> 
> Summary
> ---
> 
> Initial patch
> 
> I created a dialog that will show the RAW Bugzilla HTML output when the 
> response can't be parsed by DrKonqi. The user is encouraged to perform the 
> action again later or save the HTML data to submit a DrKonqi bug.
> 
> This function is available for the login and the submit (new report or attach 
> to an existant one) processes.
> BugzillaLib error message signals were enhanced to hold the raw html (if it 
> makes sense to show it)
> 
> The text on the dialog could be changed.
> 
> Additional fix: disconnect signals prior to creating a new report or 
> attaching to an existing one; avoiding duplicates dialog. (may be this should 
> go as another patch)
> 
> 
> This addresses bug 266542.
> http://bugs.kde.org/show_bug.cgi?id=266542
> 
> 
> Diffs
> -
> 
>   drkonqi/CMakeLists.txt 87c3d24 
>   drkonqi/bugzillalib.h cb7da10 
>   drkonqi/bugzillalib.cpp 348fb72 
>   drkonqi/reportassistantdialog.cpp 607dd63 
>   drkonqi/reportassistantpages_bugzilla.h 8681f22 
>   drkonqi/reportassistantpages_bugzilla.cpp e896c52 
>   drkonqi/reportinterface.h a1fc78a 
>   drkonqi/reportinterface.cpp a484d11 
> 
> Diff: http://git.reviewboard.kde.org/r/100681/diff
> 
> 
> Testing
> ---
> 
> Tested by faking some errors during the login and submit (new report) 
> processes.
> The code should be reviewed as I stop coding for KDE several months ago.
> 
> 
> Screenshots
> ---
> 
> Unhandled Error Dialog
>   http://git.reviewboard.kde.org/r/100681/s/77/
> 
> 
> Thanks,
> 
> Darío Andrés
> 
>



Re: git workflow draft

2011-02-19 Thread Matthias Fuchs
Am Donnerstag 17 Februar 2011, 13:46:04 schrieb Johannes Sixt:
> Am 2/17/2011 12:10, schrieb Andreas Hartmetz:
> > On Thursday 17 February 2011 09:01:05 Johannes Sixt wrote:
> >> When you develop a new feature, it is a very important choice on which
> >> version of the software you base it on. I am advocating to base a
> >> feature on a well-known, stable state. If you choose to base the
> >> feature on "today's master", you are actually building a house on
> >> moving grounds. How can you be sure that your product will be stable?
> >> 
> >> The corollary is that a new topic should be based on a project state
> >> that is no younger than necessary (to have all prerequisites that you
> >> need), so that you can be sure that it was already tested by a as many
> >> people as possible. Ideally, today, every feature targeted for 4.7
> >> should be based on the 4.6.0 release tag, whenever possible.
> > 
> > As somebody who usually runs master, I see a bit of a problem here.
> > Everybody who is working on a feature will not be testing master
> > (aspects related and unrelated to the branch) while doing so. This may
> > or may not turn out to be a problem in practice.
> 
> You do have a point here. However, I believe that it should not be a
> problem in practice:
> 
> You start a topic from a stable state that is not today's master. You
> develop the topic for a few days, or a week or two. During this time, you
> do diligent testing, and at the end you are reasonably sure to have a new
> good state. The goal here is that your testing does not suffer from
> unrelated breakages introduced by other topics that are in flight.
> 
> At this point, you want to make sure the topic is good when it's merged
> into master. Do that. And repeat all tests.
> 
> Now it's time to publish the topic. You do so by pushing the topic branch
> (not the merge!). You encourage others to merge the topic into their
> *private* repositories to get wider exposure and testing.
> 
> Yourself, you keep running the merged state, of course. You would also do
> the regular merges from upstream that you used to do. See? You are still
> testing master. If you like, you would merge topic branches offered by
> others to test their topics as well.
> 
> When your topic has cooked sufficiently long, you would repeat the merge
> into today's master (remember: do not expose your own private, messy merge
> history to others), and push the result upstream. Now the topic is in
> master.
> 
> > Maybe we'll find rules of thumb to choose the right base for topic
> > branches. Like "usually latest stable unless you rely on features of
> > master" or "usually master unless changes in master would interfere with
> > your work"... A checklist with obvious and not so obvious arguments for
> > / against choosing a particular base could also help make decisions on a
> > case-by-case basis.
> 
> If the project hygenie that I sketched above were applied universally,
> today's master would actually be a reasonable choice (contrary to what I
> said earlier).
> 
> Until then, a relaxed rule is probably sufficient: Choose a starting point
> that is convenient; but DO NOT CHANGE IT once you have done serious
> development, because a change (aka rebase) basically invalidates all your
> tests.
> 
> And common sense says: If you are cleaning up your messy topic history
> using git-rebase in such a way that you have to re-test every new commit
> anyway, you CAN choose a new starting point.
> 
> -- Hannes

I don't really agree.
Often you don't really depend on master, _but_ master will be the next stable 
release.
There can be many interdependencies which would not show up if you based your 
work on the current stable branch. And as a feature would never be added to 
the stable branch anyway I see no reason to work on a branch of it for a 
feature.
What would that gain me?

If I base it on master in fact errors could turn up which have nothing to do 
with the feature. But would that be bad? Wouldn't that rather result in faster 
fixes in bugs, no matter where they are? Wouldn't that result in a better next 
stable version as errors turn up earlier?


Re: KJob::Capabilities and KWidgetJobTracker

2011-02-09 Thread Matthias Fuchs
Am Mittwoch 09 Februar 2011, 19:59:34 schrieb Kevin Ottens:
> On Wednesday 9 February 2011 19:21:26 Matthias Fuchs wrote:
> > Am Mittwoch 09 Februar 2011, 19:06:41 schrieb Kevin Ottens:
> > > On Wednesday 9 February 2011 18:52:57 Matthias Fuchs wrote:
> > > > I wonder wether setting capabilties for KJobs should be "required".
> > > > Currently you are presented with a "Pause" button in
> > > > KWidgetJobTracker, even if the job does _not_ have the Suspendable
> > > > capability.
> > > 
> > > Isn't it more a problem of the tracker not querying the capabilities()
> > > when it gets passed a job and adapting it's UI accordyingly? That's
> > > actually one of the intended use cases for the capabilities() method in
> > > KJob.
> > 
> > Partially, but what happens if the capabilities change?
> 
> Right now, none of our jobs change capabilities once started. We never had
> the need previously, now we can extend that if needed. In which case the
> signal approach you propose seems sound to me.
> 
> Regards.

Ok.
Would a patch -- if I find the time -- including proper Capabilities support 
for the KWidgetJobTracker be welcome?


Re: KJob::Capabilities and KWidgetJobTracker

2011-02-09 Thread Matthias Fuchs
Am Mittwoch 09 Februar 2011, 19:06:41 schrieb Kevin Ottens:
> On Wednesday 9 February 2011 18:52:57 Matthias Fuchs wrote:
> > I wonder wether setting capabilties for KJobs should be "required".
> > Currently you are presented with a "Pause" button in KWidgetJobTracker,
> > even if the job does _not_ have the Suspendable capability.
> 
> Isn't it more a problem of the tracker not querying the capabilities() when
> it gets passed a job and adapting it's UI accordyingly? That's actually
> one of the intended use cases for the capabilities() method in KJob.
> 

Partially, but what happens if the capabilities change?
E.g. if you are downloading a file you can't always be certain if resuming -- 
and thus suspending -- will work when starting the job.


KJob::Capabilities and KWidgetJobTracker

2011-02-09 Thread Matthias Fuchs
Hi,

I wonder wether setting capabilties for KJobs should be "required".
Currently you are presented with a "Pause" button in KWidgetJobTracker, even 
if the job does _not_ have the Suspendable capability.

Imo what would be nicer is jobs emiting a signal capabilitiesChanged to which 
all the graphical trackers -- be it the ones provided by Plasma or whatever -- 
connect and then change the ui accordingly. E.g. displaying a Pause button for 
jobs that support suspending.

Now one backdraw could be code assuming the current behaviour --> i.e. 
ignoring capabilities to some (KWidgetJobTracker) degree.

What do you think? Are there some other backdraws I have not thought of?


Re: Multiple QRegExp crashes when multithreading in KRunner

2011-01-16 Thread Matthias Fuchs
Am Samstag 15 Januar 2011, 15:08:28 schrieb Thiago Macieira:
> On Saturday, 15 de January de 2011 12:28:38 Ingo Klöcker wrote:
> > On Friday 14 January 2011, Thiago Macieira wrote:
> > > On Friday, 14 de January de 2011 21:49:01 Ingo Klöcker wrote:
> > > > On Friday 14 January 2011, Thiago Macieira wrote:
> > > > > On Friday, 14 de January de 2011 13:46:19 Sebastian Trueg wrote:
> > > > > > However, the query parser still uses static QRegExp objects
> > > > > > which seems a bad idea, isn't that right?
> > > > > 
> > > > > static QRegExp are a bad idea regardless of whether threading is
> > > > > involved or not.
> > > > 
> > > > Why?
> > > 
> > > Because it's static. Static non-PODs aren't a good idea.
> > 
> > Where static means file static, right?
> 
> File-static, class-static or function-static, though function-static is the
> least evil of those.

Again another crash -- with Qt 4.6.2 -- that should not happen if I understood 
everything correctly:

term.contains(QRegExp("^[a-zA-Z0-9\\-\\.]+\\.[a-zA-Z]{2,6}"))

http://websvn.kde.org/tags/KDE/4.4.2/kdebase/workspace/plasma/generic/runners/locations/locationrunner.cpp?view=markup
 
line 99
https://bugs.kde.org/show_bug.cgi?id=247117

The code has changed since then (for KDE 4.6), so asking users to retry with a 
newer KDE version would not get valuable information.
Still I tried to reproduce the crash with the old code and with Qt 4.7.1 but 
was not able, so is that fixed there?


Re: Multiple QRegExp crashes when multithreading in KRunner

2011-01-15 Thread Matthias Fuchs
Am Samstag 15 Januar 2011, 15:08:28 schrieb Thiago Macieira:
> On Saturday, 15 de January de 2011 12:28:38 Ingo Klöcker wrote:
> > On Friday 14 January 2011, Thiago Macieira wrote:
> > > On Friday, 14 de January de 2011 21:49:01 Ingo Klöcker wrote:
> > > > On Friday 14 January 2011, Thiago Macieira wrote:
> > > > > On Friday, 14 de January de 2011 13:46:19 Sebastian Trueg wrote:
> > > > > > However, the query parser still uses static QRegExp objects
> > > > > > which seems a bad idea, isn't that right?
> > > > > 
> > > > > static QRegExp are a bad idea regardless of whether threading is
> > > > > involved or not.
> > > > 
> > > > Why?
> > > 
> > > Because it's static. Static non-PODs aren't a good idea.
> > 
> > Where static means file static, right?
> 
> File-static, class-static or function-static, though function-static is the
> least evil of those.

I am not sure if you missed my previous mail, though the following code also 
lead to a crash in Qt 4.6.3:

QStringList KGetRunner::parseUrls(const QString &text) const
{
  ...
  QRegExp re("\\b\\S+");
  int i = re.indexIn(text);
  ...
}

I wonder what could have caused this -- as I did not experience this myself.

http://websvn.kde.org/tags/KDE/4.4.5/kdenetwork/kget/plasma/runner/kgetrunner.cpp?revision=1142811&view=markup
Crash happens at line 114, also see 
https://bugs.kde.org/show_bug.cgi?id=255860

Is there anything I could do about this, or might it be that this problem does 
not occur with later Qt versions?


Re: Multiple QRegExp crashes when multithreading in KRunner

2011-01-14 Thread Matthias Fuchs
Am Freitag 14 Januar 2011, 01:16:07 schrieb Thiago Macieira:
> On Thursday, 13 de January de 2011 15:08:06 Aaron J. Seigo wrote:
> > On Thursday, January 13, 2011, Thiago Macieira wrote:
> > > On Thursday, 13 de January de 2011 22:43:28 Matthias Fuchs wrote:
> > > > Hi,
> > > > 
> > > > There are some crashes related to QRegExp and multithreading, so
> > > > they
> > > > appear when using KRunner.
> > > 
> > > There should be no crashes related to QRegExp and threading since Qt
> > > 4.4.
> > > If you find something, it's usually because you used the same QRegExp
> > > object to conduct searches at the same time.
> > 
> > yes, that's exactly what's happening :) Matthias already fixed one
> > krunner plugin that was doing this, but not he's faced with the
> > Nepomuk::Query::QueryParser bits.
> > 
> > as it is internal to Nepomuk and it is using static members internal to
> > the library (meaning krunner or any other app can't just use a
> > "different Nepomuk object") QThreadStorage seems the easiest solution
> > here.
> 
> Nah.
> 
> Just create a copy on the stack:
> 
> QRegExp rx = theRx;
> if (rx.indexIn(str) != 0) { ... }

Yet that is what is happening in the KGet runner case [1] -- I simply create 
an instance of QRegExp on the stack -- and there it crashes too.

Now I understand that you mentioned to make a test case, though I can't 
reproduce this crash myself.
In the report Qt 4.6.3 is mentioned.

[1] 
http://websvn.kde.org/tags/KDE/4.4.5/kdenetwork/kget/plasma/runner/kgetrunner.cpp?revision=1142811&view=markup
Crash happens at line 114, also see 
https://bugs.kde.org/show_bug.cgi?id=255860


Multiple QRegExp crashes when multithreading in KRunner

2011-01-13 Thread Matthias Fuchs
Hi,

There are some crashes related to QRegExp and multithreading, so they appear 
when using KRunner.

One happens in Nepomuk::Query::QueryParser::parse, a static method that uses 
globally defined QRegExps. [1]

Now I wonder what the best solution is for this specific problem.
Last week I discussed with Aaron and he suggested that maybe QThreadStorage 
could be used or checking if the calling thread is different from the thread 
the object lives in.
I am not sure how ThreadWeaver works, so if using QThreadStorage would 
actually make sense or if threads are quite often disposed and nothing could 
be reused in a multithreading scenario -- though still in a one thread one.

Another bug also related to QRegExp, this time the problem is in KGet [2]. 
Since I am no that experienced in multi-threadded coding it'd be great if you 
could tell me exactly how to fix that problem so that I could do it myself 
from then on without wasting your time. :)
And as far as I can remember there are further KRunner crashes attributed to 
this, so it would make KRunner more stable which would be great.

PS.: Sorry Aaron for not writing earlier, the last days were more stressful 
than anticipated.

[1] https://bugs.kde.org/show_bug.cgi?id=252652
kdelibs/nepomuk/query/queryparser.cpp

[2] kget/plasma/runner/kgetrunner.cpp:119 (the report mentiones 114, though 
that is from an older revision)


Re: Review Request: Adds method to KBookmarkManager disable showing dialogs on errors.

2010-11-30 Thread Matthias Fuchs

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6004/
---

(Updated 2010-11-30 20:57:03.986026)


Review request for kdelibs and David Faure.


Changes
---

Messed up the last diff, this one is correct now.


Summary
---

Adds method to KBookmarkManager disable showing dialogs on errors.
This way it is possible to deactivate those messages if KBoomarkManager is not 
run in the gui thread.
BUG:207592


This addresses bug 207592.
https://bugs.kde.org/show_bug.cgi?id=207592


Diffs (updated)
-

  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.h 1202414 
  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.cc 1202414 

Diff: http://svn.reviewboard.kde.org/r/6004/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Adds method to KBookmarkManager disable showing dialogs on errors.

2010-11-30 Thread Matthias Fuchs

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6004/
---

(Updated 2010-11-30 20:55:09.801653)


Review request for kdelibs and David Faure.


Changes
---

Adds a error signal that contains the translated error message.
To be able to emit it a const_cast is needed in saveAs, which we -- Albert and 
me -- are not that happy about.  :)


Summary
---

Adds method to KBookmarkManager disable showing dialogs on errors.
This way it is possible to deactivate those messages if KBoomarkManager is not 
run in the gui thread.
BUG:207592


Diffs (updated)
-

  /trunk/KDE/kdelibs/kate/plugins/autobrace/ktexteditor_autobrace.desktop 
1201809 
  /trunk/KDE/kdelibs/kate/syntax/katehighlight.cpp 1201809 
  /trunk/KDE/kdelibs/kdecore/io/kzip.cpp 1201809 
  /trunk/KDE/kdelibs/kdecore/kernel/kglobal.h 1201809 
  /trunk/KDE/kdelibs/kdecore/kernel/kglobal.cpp 1201809 
  /trunk/KDE/kdelibs/kdecore/kernel/kstandarddirs.cpp 1201809 
  /trunk/KDE/kdelibs/kdecore/localization/all_languages.desktop 1201809 
  /trunk/KDE/kdelibs/kdecore/tests/kstandarddirstest.cpp 1201809 
  /trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonlistdelegate_p.h 
1201809 
  /trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonlistdelegate_p.cpp 
1201809 
  /trunk/KDE/kdelibs/kdeui/shortcuts/kglobalaccel.cpp 1201809 
  /trunk/KDE/kdelibs/kdeui/util/kpixmapsequence.h 1201809 
  /trunk/KDE/kdelibs/kdeui/util/kpixmapsequence.cpp 1201809 
  /trunk/KDE/kdelibs/kdeui/widgets/kmultitabbar.cpp 1201809 
  /trunk/KDE/kdelibs/kdewebkit/kwebpage.cpp 1201809 
  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.h 1201809 
  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.cc 1201809 
  /trunk/KDE/kdelibs/kio/kfile/kfilemetadatawidget.h 1201809 
  /trunk/KDE/kdelibs/kio/kfile/kfilemetadatawidget.cpp 1201809 
  /trunk/KDE/kdelibs/kio/kio/accessmanager.cpp 1201809 
  /trunk/KDE/kdelibs/kio/kio/accessmanagerreply_p.cpp 1201809 
  /trunk/KDE/kdelibs/kio/kio/kfilemetainfo.cpp 1201809 
  /trunk/KDE/kdelibs/nepomuk/query/query.cpp 1201809 
  /trunk/KDE/kdelibs/nepomuk/utils/utils.cpp 1201809 
  /trunk/KDE/kdelibs/plasma/widgets/label.cpp 1201809 
  /trunk/KDE/kdelibs/plasma/widgets/pushbutton.cpp 1201809 
  /trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1201809 
  /trunk/KDE/kdelibs/plasma/widgets/tabbar.cpp 1201809 
  /trunk/KDE/kdelibs/solid/solid/CMakeLists.txt 1201809 
  /trunk/KDE/kdelibs/solid/solid/backends/udev/udevblock.h PRE-CREATION 
  /trunk/KDE/kdelibs/solid/solid/backends/udev/udevblock.cpp PRE-CREATION 
  /trunk/KDE/kdelibs/solid/solid/backends/udev/udevdevice.cpp 1201809 
  /trunk/KDE/kdelibs/solid/solid/backends/udev/udevmanager.cpp 1201809 

Diff: http://svn.reviewboard.kde.org/r/6004/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Adds method to KBookmarkManager disable showing dialogs on errors.

2010-11-30 Thread Matthias Fuchs


> On 2010-11-29 00:08:22, Albert Astals Cid wrote:
> > Why did you change the if/else logic? Also you added another kError when 
> > there was an existing one already.
> > 
> > Also as commented on IRC maybe it would make sense to emit a signal to warn 
> > whoever might be interested that an error happened
> 
> Matthias Fuchs wrote:
> I changed that to always have a useful debug output. The second one is 
> there to make it easier if there are bug reports, as it is hard to work with 
> translated messages, especially if they are quite important. So not the whole 
> message is translated, probably I should us an error code there, as 
> file.errorString() is translated.

Btw. on the signal.

Should it just be an "void error()" signal or a "void 
error(translatedErrorMessage)" one?


- Matthias


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6004/#review9034
---


On 2010-11-28 21:00:30, Matthias Fuchs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6004/
> ---
> 
> (Updated 2010-11-28 21:00:30)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> ---
> 
> Adds method to KBookmarkManager disable showing dialogs on errors.
> This way it is possible to deactivate those messages if KBoomarkManager is 
> not run in the gui thread.
> BUG:207592
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.h 1201809 
>   /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.cc 1201809 
> 
> Diff: http://svn.reviewboard.kde.org/r/6004/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthias
> 
>



Re: Review Request: Adds method to KBookmarkManager disable showing dialogs on errors.

2010-11-29 Thread Matthias Fuchs


> On 2010-11-29 00:08:22, Albert Astals Cid wrote:
> > Why did you change the if/else logic? Also you added another kError when 
> > there was an existing one already.
> > 
> > Also as commented on IRC maybe it would make sense to emit a signal to warn 
> > whoever might be interested that an error happened

I changed that to always have a useful debug output. The second one is there to 
make it easier if there are bug reports, as it is hard to work with translated 
messages, especially if they are quite important. So not the whole message is 
translated, probably I should us an error code there, as file.errorString() is 
translated.


- Matthias


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6004/#review9034
---


On 2010-11-28 21:00:30, Matthias Fuchs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6004/
> ---
> 
> (Updated 2010-11-28 21:00:30)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> ---
> 
> Adds method to KBookmarkManager disable showing dialogs on errors.
> This way it is possible to deactivate those messages if KBoomarkManager is 
> not run in the gui thread.
> BUG:207592
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.h 1201809 
>   /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.cc 1201809 
> 
> Diff: http://svn.reviewboard.kde.org/r/6004/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthias
> 
>



Re: Review Request: Adds method to KBookmarkManager disable showing dialogs on errors.

2010-11-28 Thread Matthias Fuchs

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6004/
---

(Updated 2010-11-28 21:00:30.131444)


Review request for kdelibs and David Faure.


Changes
---

Changed the name of the method to setDialogsAllowed.


Summary
---

Adds method to KBookmarkManager disable showing dialogs on errors.
This way it is possible to deactivate those messages if KBoomarkManager is not 
run in the gui thread.
BUG:207592


Diffs (updated)
-

  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.h 1201809 
  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.cc 1201809 

Diff: http://svn.reviewboard.kde.org/r/6004/diff


Testing
---


Thanks,

Matthias



Review Request: Adds method to KBookmarkManager disable showing dialogs on errors.

2010-11-28 Thread Matthias Fuchs

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6004/
---

Review request for kdelibs and David Faure.


Summary
---

Adds method to KBookmarkManager disable showing dialogs on errors.
This way it is possible to deactivate those messages if KBoomarkManager is not 
run in the gui thread.
BUG:207592


Diffs
-

  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.h 1201809 
  /trunk/KDE/kdelibs/kio/bookmarks/kbookmarkmanager.cc 1201809 

Diff: http://svn.reviewboard.kde.org/r/6004/diff


Testing
---


Thanks,

Matthias



Re: Review Request: Make sure to remove KXMLGUIClients from factory on destruction

2010-11-17 Thread Matthias Fuchs


> On 2010-07-03 22:44:14, David Faure wrote:
> > I was initially against this kind of thing (too many problems with 
> > "intelligent" destructors), this is why it hasn't been done before.
> > But after long consideration, I think this one is safe and, as you say, 
> > somewhat expected by users of the framework, so I'm ok with it.
> > I'll commit it to trunk, at least, we'll see if it proves stable enough for 
> > 4.5.0.
> > Thanks a lot for the patch, and sorry for the delay.
> 
> David Faure wrote:
> Turns out I was right, the "intelligent destructor" creates trouble - bug 
> 246652 :-)
> I think I'll make it a bit less intelligent (it should make the factory 
> forget this client, but it should not unplug actions one by one).
> 
> Thomas Friedrichsmeier wrote:
> Hm. I didn't look too closely at what ContainerNode::destruct() does. But 
> on first glance this sounds like there will be a memory leak if a client is 
> deleted without being removed, first, now.
> 
> *If* so, that's still better than causing a seemingly unrelated crash a 
> long time later. But perhaps there should be a debug message telling 
> developers to fix their object destruction order, whenever a client still has 
> a factory while being deleted (and for KDE 5, we might consider reducing the 
> smartness even further to simply crashing right there and then).

Forgive me my complete ignorance -- as I haven't looked at the code -- but 
couldn't smart pointers solve some of the problems encountered here?


- Matthias


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/4126/#review6354
---


On 2010-06-01 07:15:25, Thomas Friedrichsmeier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/4126/
> ---
> 
> (Updated 2010-06-01 07:15:25)
> 
> 
> Review request for kdelibs, Christoph Feck and David Faure.
> 
> 
> Summary
> ---
> 
> Under some circumstances, dangling pointers to deleted KXMLGUIClients may 
> remain in the list of a KXMLGUIFactory's clients. Usually, KXMLGUIClients are 
> also KParts, and managed by a KPartManager. If everything goes as expected, 
> the KPartManager detects when the active KPart is destroyed, and the 
> KParts::MainWindow takes care of removing it from the factory. When 
> XMLGUIClients are nested, things may not always go as smoothly. 
> KXMLGUIFactory::removeClient() removes clients recursively, however, if one 
> of the child clients gets destroyed before the parent KParts was removed from 
> the factory, it will be taken out of the parent client's list of children, 
> but not out of the factory's list of clients. Possibly, there are other 
> subtle cases, where dangling pointer remain in KXMLGUIFactory::clients(). 
> Naturally this will lead to crashes, when code tries to do something with all 
> clients of the factory. The listed bugs are symptoms of this cause (at least 
> potentially; with non-reproducible bugs it's always hard to tell, whether 
> there was only a single cause).
> 
> Probably this issue is addressable in the respectively parts / applications, 
> by making sure that any child clients are not deleted before the toplevel 
> part is removed from the factory. However, it is possible, and sensible to 
> add code in the KXMLGUIClient d'tor to make sure that the factory will never 
> keep dangling pointers. This patch does so. Some additional code is needed to 
> make sure that this does not cause trouble during application exit, by making 
> sure that the KXMLGUIFactory is deleted before the KXMLGUIBuilder, and 
> remaining clients no longer assume a factory.
> 
> Note:
> Bug #170806 contains a valgrind log, and reproducible instructions on 
> crashing kontact, which can probably be mapped to other affected applications.
> 
> 
> This addresses bugs 170806, 183338, 205625, and 212397.
> https://bugs.kde.org/show_bug.cgi?id=170806
> https://bugs.kde.org/show_bug.cgi?id=183338
> https://bugs.kde.org/show_bug.cgi?id=205625
> https://bugs.kde.org/show_bug.cgi?id=212397
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/kdeui/tests/kxmlgui_unittest.h 1129152 
>   trunk/KDE/kdelibs/kdeui/tests/kxmlgui_unittest.cpp 1129152 
>   trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiclient.cpp 1129152 
>   trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguifactory.cpp 1129152 
>   trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiwindow.cpp 1129152 
> 
> Diff: http://svn.reviewboard.kde.org/r/4126/diff
> 
> 
> Testing
> ---
> 
> Fixes crashes during toolbar / shortcut configuration for RKWard.
> 
> Results of existing unit tests are not affected. New unit test added.
> 
> Tested on trunk, only. If this should go into 4.4, I'd like someone else to 
> do the commit on the branc

Re: "Cornelius's grand plan" - Merging KDElibs into Qt

2010-11-01 Thread Matthias Fuchs
Am Montag 01 November 2010, 11:53:01 schrieb Sean Harmer:
[...]
> One other thing that I have found that could be prohibiting wider adoption
> of KDE as a development platform is the lack of coherent documentation.
> Please do not take this as a criticism, techbase/userbase/api docs are all
> excellent resources and are constantly improving. From experience of
> trying to get some colleagues to utilise the KDE libraries who had not
> been exposed to them before, a large hurdle was discoverability of the
> APIs. A lot of the information is there it's just a question of finding
> it. If information is difficult or time consuming to find this discourages
> developers from coming back again or from branching out and using other
> facilities in the future. I do not pretend to have the answers here. KDE
> is a huge project and for the most part developed by volunteers and godo
> documentation is very difficult and hugely expensive in terms of
> time/blood/sweat/tears to write. Kudos to all the documentation writers
> (and devs/artists/testers etc).

I agree on that.
I often don't know if something I try to do exists already in kdelibs and then 
you can see me asking on irc and fantastic people like dfaure answer me. 
Despite how great that has been for me this is really not a good path for a 
wider kdelibs adoption.
Maybe a good overview website could help here. There the different classes are 
split into groups and you find out fast what they are for.

Yeah writing such an overview will probably be very hard in itself, since 
kdelibs is very large.
In that regard it is kinda "refreshing" -- but also demotivating since that 
issue has not been solved -- to see that we are not the only ones with such 
problems, e.g. imo boost faces the exact same problem. Boost is so large that 
it is quite hard to grasp what all you can do with it. And even if you know 
what you can do, often it is not clear if that would be a "good" -- e.g. fast 
-- path to go.


Re: "Cornelius's grand plan" - Merging KDElibs into Qt

2010-11-01 Thread Matthias Fuchs
Am Sonntag 31 Oktober 2010, 18:28:50 schrieb todd rme:
> On Sun, Oct 31, 2010 at 1:24 PM, Albert Astals Cid  wrote:
> > A Diumenge, 31 d'octubre de 2010, todd rme va escriure:
> > > On Sun, Oct 31, 2010 at 9:26 AM, Michael Jansen  > 
> > jansen.biz>wrote:
> > > > On Sunday 31 October 2010 12:33:22 Mark Kretschmann wrote:
> > > > > Hey all,
> > > 
> > > I think this sounds like the place to start, for several reasons:
> > > 
> > > 5. Licensing shouldn't be as much of a problem
> > 
> > How come?
> > 
> > Abert
> 
> 1. Because there is unlikely to be as many developers with rights to the
> code
> 2. Because it would be easier to re-implement the changes if need be, since
> they are smaller
> 
> -Todd

And what is with
"3. Developers not agreeing with the licensing at all"?

I suppose this is what aacid was refering to.
And as he has mentioned before he is one that opposes such a move, with good 
reasons.

And no a svn blame would not be enough, you would have to track down the whole 
history. Because derivative work would not be allowed!

Who would track that all down and who would rewrite code that would need 
rewriting?

Imo this would end up as a waste of time, better to modularize KDE on its own, 
than to move as much as possible into Qt.


Re: "Cornelius's grand plan" - Merging KDElibs into Qt

2010-11-01 Thread Matthias Fuchs
Am Sonntag 31 Oktober 2010, 13:45:48 schrieb Matt Williams:
> On 31 October 2010 12:37, Modestas Vainius  wrote:
> > Hello,
> > 
> > On sekmadienis 31 Spalis 2010 14:04:28 Matt Williams wrote:
> >> On 31 October 2010 11:53, John Tapsell  wrote:
> >> > On 31 October 2010 11:33, Mark Kretschmann  wrote:
> >> >> Hey all,
> >> >> 
> >> >> after reading the whole thread that started with Chani's mail ("why
> >> >> kdelibs?"), I think the noise level has become a bit too much there.
> >> >> Cornelius had proposed this rather daring idea:
> >> >> 
> >> >> http://lists.kde.org/?l=kde-core-devel&m=128842761708404&w=2
> >> > 
> >> > Sounds great.  This should probably be done by picking a specific
> >> > technology in KDE, and adapting it and merging it to work in Qt.
> >> > 
> >> > A wonderful place to start would be kioslaves imho.  This is something
> >> > which has a real advantage, is relatively self-contained, and would
> >> > provide a big advantage.  Possibly it needs to be merged more with the
> >> > Qt API though.
> >> 
> >> So, if we decided that we wanted to merge KIO slaves into Qt, what
> >> would the steps we go through be? If we're going to be doing this with
> >> a number of classes we need to have a process which ensures that the
> >> code is Qute enough, KDE still compiles against it (with minimal/no
> >> code changes) etc.
> > 
> > With my packager hat on:
> > 
> > But what happens when you (KDE) decide that you really need a new feature
> > of kioslaves for the next release. But the next Qt release is not due in
> > 7 months or you (again) have trouble merging changes back to Qt with
> > patience running out? Sure, developers compiling from source can patch
> > Qt, install differently configured builds to different prefixes
> > with/without specific features etc. But what are poor packagers supposed
> > to do? Phonon situation all over again? It was *really* bad and my mind
> > is still recovering from that experience. I really doubt I'm ever going
> > to trust such merges to Qt again.
> > 
> > In my opinion, KDE should keep libraries small, *modular* and develop
> > them in- house. Try not to attach binaries and esp. daemons to the
> > libraries. Maybe strip KDE branding from library names to gain wider
> > acceptance. Only this way you as a project will be in control of release
> > schedules and feature sets.
> 
> Basically, what you're suggesting is to move as much (well, within
> reason of course) of kdelibs 'upstream' into kdesupport. I think I
> agree that this could be the way to go (at least for the short/medium
> term).
> 
> It doesn't solve our BIC/SIC problem (unless as I asked before: does
> moving the real code out into kdesupport and keeping wrapper classes
> in kdelibs solve either of those?) so we'd still need to make this a
> KDE5 thing. Or would we just have to make it libkdecore.so.6 bump
> while keeping KDE Platform/SC at version 4? It would be breaking our
> BC promise but maybe we could get away with it? :)

I am not sure but I don't see why this should cause problems, as long as the 
versions in kdesupport are in a different namespace or use a different naming. 
And as all (sic?) the clases in kdelibs use d-ptrs this should in my 
uneducated  opinion just work.

Still having wrappers for everything would be a lot of work. Who would 
maintain something like that? I mean imagine fowarding one signal was 
forgotten, what mess this could cause, even if a lot of unitests were written 
I doubt this would work flawless.

So wrapper clases might appear like a nice intermediate step, but imo they 
would add more burden than help.