Version: 1.4 post (using KDE 3.4.3, compiled sources) Compiler: gcc version 3.3.3 (NetBSD nb3 20040520) OS: NetBSD (amd64) release 3.0_BETA I just fixed bug #117252, but various other cases of autofill (still?) don't work correctly. For example dragging a date from bottom up has quite fancy results. Time doesn't work properly either. Most common/standard types of autofill work correctly though. I suggest to make a thorough investigation what works and what does not, fixing things along the way. IMO this would be a good candidate for a decent automated test (unit testing or similar). So maybe it's even wise to write a test case before investigating, which will automatically reveal all (tested) errors. Relevant source code: Sheet::autoFill() Sheet::FillSequenceWithCopy() most of this is in kspread_autofill.cc
SVN commit 485336 by raphael: Time is now incremented by one hour by default (1/24) KSpread::Cell::copyFormat now also copies the value format (!!!) This is necessary to correctly display "dragged down" values that actually use the Generic format instead of a determined format. update CHANGES As far as I can see, autofill now works correctly EXCEPT for bottom-up dragging. CCBUG: 117252 CCBUG: 117619 M +6 -0 CHANGES M +22 -8 kspread_autofill.cc M +5 -0 kspread_cell.cc --- trunk/koffice/kspread/CHANGES #485335:485336 @@ -7,7 +7,13 @@ - Prevent hiding all rows/columns (#483630) - Add a lot of QWhatsThis help to various dialogs - Handbook update for some dialogs +- Implement move sheets +- Fix date increment for autofill (#117252) +Some important developer visible changes: +- Decouple KSpread::Cell and KSpread::Format (association instead of inheritance) +- Provide setter and getter methods for KSpread::Point + since 1.4.1 =========== - Fix slow scrolling left/right (#110551, #101234) --- trunk/koffice/kspread/kspread_autofill.cc #485335:485336 @@ -1,5 +1,6 @@ /* This file is part of the KDE project + Copyright 2005 Raphael Langerhorst <raphael.langerhorst@kdemail.net> Copyright 2002-2004 Ariya Hidayat <ariya@kde.org> Copyright 2002-2003 Norbert Andres <nandres@web.de> Copyright 2002 John Dailey <dailey@vt.edu> @@ -1057,21 +1058,34 @@ { if ( _srcList.at( s )->isFormula() ) { - QString d = _srcList.at( s )->encodeFormula(); - cell->setCellText( cell->decodeFormula( d ) ); + QString d = _srcList.at( s )->encodeFormula(); + cell->setCellText( cell->decodeFormula( d ) ); } else if(_srcList.at( s )->value().isNumber() && _srcList.count()==1) { - double val; - if ( _srcList.at( s )->formatType() == Percentage_format ) - factor = 0.01; + double val; + int format_type = _srcList.at( s )->formatType(); + if ( format_type == Percentage_format ) + { + factor = 0.01; // one percent + } + else if ( _srcList.at( s )->isTime() ) + { + // FIXME this is a workaround to avoid those nasty one minute off + // "dragging down" time is inaccurate overa large lists! + // This is the best approximation I could find (raphael) +// factor = 1.000002/24. + 0.000000001; + factor = 0.041666751; + } + if (!down) val = (_srcList.at( s )->value().asFloat() - (incr * factor)); else val = (_srcList.at( s )->value().asFloat() + (incr * factor)); - QString tmp; - tmp = tmp.setNum(val); - cell->setCellText( tmp ); + + QString tmp; + tmp = tmp.setNum(val); + cell->setCellText( tmp ); ++incr; } else if((AutoFillSequenceItem::month != 0L) --- trunk/koffice/kspread/kspread_cell.cc #485335:485336 @@ -1,5 +1,6 @@ /* This file is part of the KDE project + Copyright 2005 Raphael Langerhorst <raphael.langerhorst@kdemail.net> Copyright 2004-2005 Tomas Mecir <mecirt@gmail.com> Copyright 1999-2002,2004,2005 Laurent Montel <montel@kde.org> Copyright 2002-2005 Ariya Hidayat <ariya@kde.org> @@ -555,6 +556,10 @@ void Cell::copyFormat( int _column, int _row ) { const Cell * cell = format()->sheet()->cellAt( _column, _row ); + + Q_ASSERT(cell); + + d->value.setFormat(cell->d->value.format()); format()->setAlign( cell->format()->align( _column, _row ) ); format()->setAlignY( cell->format()->alignY( _column, _row ) );
Hello I'm the reporter of bug 117252, and first want to thank you for fixing that bug so quickly. :) [I have added a question there and request your reply.] Now I would like to know, is this: "Time is now incremented by one hour by default (1/24)" which you have written above also part of the bug? Because it is definitely *not* expected behaviour. When you enter a date and drag it, if the time also gets incremented, will not an entire date get skipped after 24 days? I mean, when a cell is configured to display only the date, either due to data format auto-detection or due to manual choice of user, the user does not see the time. So when s/he sees: 2005-12-01 [00:00:00] (bracket data below considered invisible) 2005-12-02 [01:00:00] ... 2005-12-24 [23:00:00] 2005-12-26 [00:00:00] (which is equivalent to 2005-12-25 24:00) will this not be an unexpected behaviour? IMO there is not much practical need for a list which increments the hour along with the date. If a user actually wants such a list, then s/he must type: 2005-12-01 00:00 2005-12-02 01:00 and then select both cells and *then* drag. This is how I have been creating lists with unexpected increments in the many years I have been working with spreadsheet programs. Unless the user gives two such data to imply the desired increment, the default behaviour expected (from a lay user's POV) is: (I give the display format and the corresponding expected increment) YYYY-MM-DD or DD-MM-YYYY or MM-DD-YYYY or MM-DD or DD-MM (i.e. any format whose smallest display value is the day) : one day YYYY-WW or WW-YYYY : (i.e. year and [ISO?] week of year) one week YYYY-MM or MM-YYYY : one month HH:MM:SS : one second HH:MM : one minute YYYY-MM-DD HH:MM:SS : again one second YYYY-MM-DD HH:MM : again one minute In short, the smallest visible unit is expected to increment by 1. The cases of incrementing YYYY and HH are non-distinct [?] from incrementing an ordinary number list so I did not include them above.
SVN commit 485426 by raphael: CCBUG: 117619 Fix dragging from bottom up. Use a different increment for time when dragging from bottom up (again, to avoid "off by one" values). As far as I can see "dragging" now works as expected in all respects. But there are some open questions to discuss, so I leave this bug open for now. M +10 -3 kspread_autofill.cc --- trunk/koffice/kspread/kspread_autofill.cc #485425:485426 @@ -674,7 +674,7 @@ for ( y = dest.top(); y < src.top(); y++ ) destList.append( nonDefaultCell( x, y ) ); QPtrList<Cell> srcList; - for ( y = dest.top(); y <= dest.bottom(); ++y ) + for ( y = src.top(); y <= src.bottom(); ++y ) { srcList.append( cellAt( x, y ) ); } @@ -1074,8 +1074,15 @@ // FIXME this is a workaround to avoid those nasty one minute off // "dragging down" time is inaccurate overa large lists! // This is the best approximation I could find (raphael) -// factor = 1.000002/24. + 0.000000001; - factor = 0.041666751; + if (down) + { +// factor = 1.000002/24. + 0.000000001; + factor = 0.041666751; + } + else + { //when dragging "up" the error must of course be the other way round + factor = 0.0416665; + } } if (!down)
Regarding Time increments: Date and Time are never together in one cell, you can either choose a date OR a time format. This means that date and time are not incremented together. So your first use case won't happen (where one day gets missed out). What can happen though is if you use one column for date, and the second for time. If you "drag down" this pair you will actually get an hour increment for the time as well, and it will wrap around 24:00, but NOT affecting the date - so no day get's missed out. In this case you would like to enter two rows to define the desired increment - 1 day for the date, and 0 for the time, this way you can drag down the list and get the same time for each new day. Incrementing by the smallest display value: From an object oriented point of view I would say the way a date is displayed should not affect the way it is handled internally. From a technical point of view I would say it's rather complex to achieve the desired result (as you described) because the increment for the month would be different for each month (28-31 days). To achieve a correct increment for every cell the calendar system needs to be used to get the needed increment. All this needs to deal with maybe 20 different ways of entering a date (local settings). So, this complexity is subject to bugs. Also I think this could lead to further unexpected results: consider the following situation (in case above is implemented)... C1: 02-2005 You drag that down and get: C2: 03-2005 C3: 04-2005 and so on But if you have: C1: 02-2005 C2: 03-2005 and you select C1:C2 and drag that down, you get: C3: 03-2005 C4: 04-2005 surprised? Well, at least unexpected, right? Because March is in there twice. The reason is that this years February only had 28 days - so this is taken as THE increment (again, dates are just "numbers"). On the other hand, the difference is "one month", but what should KSpread do? Should it treat it as a plain number and increment with the given step? Or should it "intelligently" choose 1 month as increment? [Yes, I know, the +1 increment for plain numbers is not yet the default, but I think I will change that soon, which also complies to all other spreadsheet apps known to me.] In order to avoid some confusion, I think it's best to stick to always increment by 1 if only one cell is selected, otherwise increment by the given interval or sequence if no interval can be detected. That's a rather simple rule and easier to understand than too many special cases. Personally I also prefer simpler code as it tends to be better understandable and maintainable. I think though that I see a way of implementing this at least with dates, which would not be TOO complex. What do you think?
> Regarding Time increments: > Date and Time are never together in one cell, you can either choose a date > OR a time format. This means that date and time are not incremented > together. Well, if I may use a rather strong word, this is preposterous! And to be more polite, this is rather highly unexpected! Both OOo Calc and Excel handle date and time as one. There are lots of intelligent reasons for that, one of which I can think of off-hand: May-01 01:00 May-01 02:00 dragged down to 72 times gives us a nice planner for May 01, 02 and 03. Is there a particular reason that KSpread does this differently? > Incrementing by the smallest display value: > From an object oriented point of view I would say the way a date is > displayed should not affect the way it is handled internally. OK, but from what you are saying below: it is difficult to increment a date by month just because the smallest display value is month, because the increment for a month is difficult. It may be difficult, but I say, if it is what the user expects, you must give it to him. That is good programming, no? Personally I've done one or two projects myself in other languages, and even if I only do those programs for myself, I am highly particular about these details, error checking, etc. I crack my head over the nuances of the error-checking code, since, for me, a program must be user-oriented, not object-oriented! > From a technical point of view I would say it's rather complex to achieve > the desired result (as you described) because the increment for the month > would be different for each month (28-31 days). To achieve a correct > increment for every cell the calendar system needs to be used to get the > needed increment. Well, there is always the DATE function. Only if you write extra code *here* to handle the increment, you have a problem. If you invoke the pre-existing DATE function, you do not complicate or duplicate the code at all. if format == any("yyyy-mm", "mm-yyyy" etc) then nextvalue = date(thisvalue.year; thisvalue.month+1; thisvalue.day); Over. > But if you have: > C1: 02-2005 > C2: 03-2005 > and you select C1:C2 and drag that down, you get: > C3: 03-2005 > C4: 04-2005 This won't happen if you use the method I showed you above. > treat it as a plain number and increment with the given step? Or should it > "intelligently" choose 1 month as increment? It should intelligently choose the increment. So with the above, you have if (cell2 - cell1) == (date(year(cell2); month(cell2); 1) - date(year(cell1); month(cell1); 1) then increasebymonth = true; if format == any("yyyy-mm", "mm-yyyy" etc) then increasebymonth = true; ... if increasebymonth then nextvalue = date(thisvalue.year; thisvalue.month+1; thisvalue.day); You get the idea. > [Yes, I know, the +1 increment for plain numbers is not yet the default, I don't understand what you mean by this. > In order to avoid some confusion, I think it's best to stick to always > increment by 1 if only one cell is selected, If you do this, then if a date cell is formatted as YYYY-MM and a person drags it down, then for at most 31 entries he sees the same YYYY-MM value and after that YYYY-(MM+1) is seen, and he wonders, what is going on. This is not expected behaviour. > otherwise increment by the > given interval or sequence if no interval can be detected. If "no interval can be detected" then what do you mean by "the given interval or sequence"? > Personally I also prefer simpler code as it tends to be better > understandable and maintainable. Of course, but as and when you add more feature and more intelligence to your program, the code becomes unavoidably longer, and perhaps more complex. I don't need to teach you programming. You certainly know more and are more experienced than I. I am just a linguist who does programming for a hobby. > I think though that I see a way of implementing this at least with dates, > which would not be TOO complex. Would the way I outlined about be the way you thought you saw? Best regards, Shriramana.
So you, as an user, request the intelligent way of incrementing dates. I will see what I can do. Regarding combination of date and time: Dates are represented by the whole part of a number (before the decimal point) while time is represented as a fraction of a number (1.0 is 24 hours). Thus it would be possible to reasonably combine date and time formats. But such a combined format does not yet exist. Best Regards, Raphael
I am sorry, did I offend you? Your reply was so brief. I do apologise. 1. What did you think of the idea of using the date function: 2. Please answer these two questions: > otherwise increment by the > given interval or sequence if no interval can be detected. If "no interval can be detected" then what do you mean by "the given interval or sequence"? > I think though that I see a way of implementing this at least with dates, > which would not be TOO complex. Would the way I outlined about be the way you thought you saw?
Thanks for your suggestions and apologies (yes, I felt a bit offended). Regarding interval detection: KSpread tries to find a mathematical sequence for the given input list (the selection area before starting to drag). If it finds a mathematical sequence it goes on to extend it. If it does not find such a mathematical sequence it repeats the selected sequence (= what I called the "given interval or sequence"). using DATE function: I need to look into this, whether it is possible. Your suggestions: indeed, gives me some ideas. Current state of the bug report: As the basic functionality of autofill is fixed I'm now treating this more as a whishlist (in terms of priority). So I might first fix other bugs (or work on other important things in general) before I get back to this. I do intend though to have an intelligent autofill for dates ready for the 1.5 release. Your explainations make sense to me that increasing by the smallest "visible" date part is the expected behaviour in most cases. So I/we will work towards this. Thank you for your ideas and reports. Raphael
I won't work on this any time soon. There are more important things to do at the moment. If you think it important enough for that you would like to spend time on it yourself, please provide a patch and I will apply it. Thank you.
Alternating series (1,10,2,20,3,30,...) are not handled correctly.
Can you please give us a use-case for alternating series? I myself have never used them with any spreadsheet app.
Thank you for the bug report. As this report hasn't seen any changes in 5 years or more, we ask if you can please confirm that the issue still persists. If this bug is no longer persisting or relevant please change the status to resolved.