Bug 94726

Confirmed
KDE-3.3.2 compiled from sources (gcc-3.3.3 on Fedora Core 1).

Looking into it. The bug is in
void KSnapshot::autoincFilename()
Expect a patch soon.

Created attachment 8627 [details]
Patch fixing bug and improving extension behaviour

This patch does two things:

1. Fixes the bug (i.e number is always incremented by one). I got rid of
QRegExp because it looks like QRegExp::searchRev() has a bug, i.e:

    QString name = "foo123.png";
    QRegExp rx("[0-9]+");
    int i = rx.searchRev(name); //i == 5 instead of 3 

I will ask the trolls about this.

2. Deals with extensions in a better way, by making sure that when it looks for
a number in the filename, the extension is not considered. This way, extensions
that contain a number (i.e mp3) won't get modified.

The behaviour for incrementation is now:
foo -> foo1
foo1 -> foo2
foo19 -> foo20
foo.png -> foo1.png
foo1.png -> foo2.png
foo.mp3 -> foo1.mp3
foo1.mp3 -> foo2.mp3
foo. -> foo1.
foo1. -> foo2.
.png -> 1.png
.mp3 -> 1.mp3

Note to maintainers: I left the debug statements (commented out for the release
version) in the code so that you can have a feel of the exact algorithm by
uncommenting them.

Please commit.

Current CVS does no longer show this problem

>Current CVS does no longer show this problem 

Yes it does. And 3.4.0-rc1 too.

Please reopen, it is NOT fixed.

The behaviour mentioned in comment #2 is not a bug in the QRegExp::searchRev() function (as confirmed by trolltech), it is by design, which is worse.
Please read http://bugs.kde.org/show_bug.cgi?id=100107#c9 to see exactly what  QRegExp::searchRev() does, and why it is a bogus function.

It is most certainly not what the author of ksnapshot.cpp had in mind, given that calling in reality searchRev() used with QRegExp rx("[0-9]+") is _guaranteed_ to _never_ match more than one digit (the last one).

I reiterate my call to commit my patch above, and to avoid using 
QRegExp::searchRev() altogether.

regards,

Philippe

reopened as requested

CVS commit by rich: 

Revert back to the orignal 'first number' behaviour of the filename increment.

BUG: 94726


  M +1 -1      ksnapshot.cpp   1.80


--- kdegraphics/ksnapshot/ksnapshot.cpp  #1.79:1.80
@@ -378,5 +378,5 @@ void KSnapshot::autoincFilename()
 
     // Does it have a number?
-    int start = numSearch.searchRev(name);
+    int start = numSearch.search(name);
     if (start != -1) {
         // It has a number, increment it

This fixes the 19->100 bug, but reverts to substandard 'first number' behaviour, with all its pitfalls including ignoring extensions.

You will get this kind of behaviour now:

up2date_menu_1.png -> up3date_menu_1.png
xorg-x11-devel_config_1.png -> xorg-x12-devel_config_1.png 
foo.mp3 -> foo.mp4

which not really satisfactory.

I think your change from 'first number' to 'last number' was a change for the good in terms of interface, so why not fix the way it was implemented and get it right with extensions at the same time ?

On Mon, Mar 14, 2005 at 03:19:18PM -0000, Philippe Rigault wrote:
> 
> I think your change from 'first number' to 'last number' was a change for the good in terms of interface, so why not fix the way it was implemented and get it right with extensions at the same time ?


Because a correct solution needs the user to be able to configure the
insertion point which will necessarily introduce new translation
strings.

Cheers

Rich.

Even if it is not in all points as all would like, a fixed 19>100 bug should work now makes it adaptable, workable for end-users. 
One will have to take care in file naming though, but that is usance where incremental numbers are involved. Any further refinements are appreciated of course, but beyond the intention of my bug-report..
Thanks, and now I hope I will get it implemented somehow..