Eike Rathke wrote:
Hi William,
[snip]
Method ScFormulaCell::IsMultilineResult() then should simply be
if (!IsValue())
return aResult.IsMultiline();
return false;
The IsValue() call here also assures that the cell is recalculated if
needed.
I thought some caching in this area would work, however my main concern
was in figuring out the updating mechanism. You've provided the vital
information, thanks. I'll have a go at implementing this next.
Moving the detection to ScFormulaResult has another advantage that it
can be used independently of ScFormulaCell.
Some notable changes that the patch adds is:
[... nice list snipped ...]
Some potential issues:
- Compatibility with Excel possibly needs discussing. We've not been
compatible with Excel in the past and this patch does not make it 100%
compatible, rather it improves compatibility, and improves further where
Excel gets it wrong IMHO.
I really don't have any problem with Calc being superior to Excel at
some places ;-) But being different here may result in documents being
displayed with different row heights when imported, something that
sometimes makes users act strangely..
:)
I've chosen to show the newlines by default,
whereas this is impossible in Excel unless the "Wrap text" option is
ticked in the Format Cells dialog. To me the Excel behaviour is
incorrect as the option turns on newline display as well as wrapping the
contents, so it is doing something more than what it advertises - it
mixes formatting with content display.
The behaviour in Calc showing new
lines is now consistent between ScFormulaCell and ScEditCell.
For interoperability I think we should follow the Excel approach and
only display multiline if the wrap option is set. Note that Excel sets
it automatically for a non-formula cell if that contains a linefeed. If
we wanted to automatically set the row height later we could introduce
an option; doing it the opposite way, display multiline now and turn it
off later, would be harder from a user experience's perspective.
By interoperability, do you mean importing Excel sheets or default
behaviour of Calc when editing a spreadsheet?
Just providing behaviour the same as Excel is too simplistic a solution.
The problem is more complicated. I'll attempt to explain why.
Consider a simple cell even without a formula. I'll refer to these
non-formula cells as regular cells. Lets say we have two lines typed in.
There are 3 ways to display this in a cell:
A) The natural way:
A rather long line one
A rather long line two
B) The "wrap text" way:
A rather
long line
one
A rather
long line
two
C) The single line way:
a rather long line onea rather long line two
The crux of the problem is that between Excel and Calc there is only one
option to display these three approaches, ie, "wrap text" is either on
or off. Excel does B) and C). Calc does A) and B). Incidentally Gnumeric
does A) and B). So even before we consider what sort of compatibility to
consider for formula cells, we have to solve the 3 display conflicts for
regular cells, given just 1 tick box. In a nutshell, there is no way to
have compatibility unless both Excel and Calc implement a 3rd option.
A further slight complication is that there are 3 subtly different ways
for displaying C)...
C1) The Excel 2003 way (no line separation character):
a rather long line onea rather long line two
C2) The Excel 2007 way (? in a box as separation character):
a rather long line one?a rather long line two
C3) The single line Calc way (space as separation character):
a rather long line one a rather long line two
Note that Calc can only currently show single lines for the result in
formula cells, whereas Excel can show single lines in both regular and
formula cells. Gnumeric simply cannot show the multiline in a single line.
Importantly, Excel and Gnumeric show multiline cells consistently the
same no matter if the text comes from a regular cell or the result of a
formula cell. Albeit, Excel chooses C) and Gnumeric chooses A). The
inconsistency needs fixing in Calc. My patch fixes this and displays
formula results the same as regular cells using A). The other option to
create consistency was to leave formula cell results untouched, but make
regular cells display as a single line (like Excel), it would make
multiline behaviour worse and importantly, it would break compatibility
with older versions of Calc... just not an option in my book.
There is no perfect solution as we cannot get Excel to display
multilines like Calc/Gnumeric currently do. So I propose, we take the
following compromise, which is pretty much the same approach as Gnumeric...
Cells with "wrap text" selected... easy, we make Calc display multilines
the same as all versions of Excel and Gnumeric.
For cells without "wrap text" selected... forget about working out which
of C1) C2) and C3) to go with as there is no consistency in this
approach whichever is chosen, it'll either upset at least two of old
Calc users or Excel 2003 or 2007 users. Instead, let's display the
multilines correctly as in A) above. I see the following pros and cons:
Pros
----
a) Consistent display of the same output in both regular and formula cells
b) No backward compatibility issues for old Calc spreadsheets with
multilines in regular cells.
c) Ideal behaviour for multiline regular cells and formula cells.
d) "Wrap text" consistency across all spreadsheet programs for both
regular and formula cells. Most Excel spreadsheets using multilines are
going to be using this approach so compatibility overall is good.
e) Excellent compatibility with Gnumeric would be possible (once they
finish their ODF implementation).
Cons
----
a) Backward compatibility display problem for old Calc spreadsheets with
multilines in formula cells (probably not many of these as multiline
display was well and truly broken)
b) Calc spreadsheets with multilines which get saved in xls format
without "wrap text" will look different in Excel. Note that this already
occurs for regular cells. It will now occur for both regular and formula
cells. Perhaps we can warn about which cells this will happen in?
c) Excel spreadsheets with multilines and without "wrap text" will
display differently when opened in Calc... probably not many of these as
it is a kind of broken feature.
Perhaps an important point is that the changes are unlikely to affect
the majority of spreadsheets in existence as multilines overall is not
such a common spreadsheet functionality.
b) and c) could be addressed by Calc implementing a 3rd display option,
say a "single line" option which can only be selected with "wrap text"
deselected. That way compatibility with importing Excel spreadsheets
would be pretty good (not perfect because of Excel 2003/2007
differences). A pretty good match on export would be possible but only
if the exporter knows to select the "single line" option everywhere as
the ideal export matching would only be possible if Excel also
implemented the "single line" display option. Further discussion on the
"single line" option is probably needed, but personally, I don't think
it will add much with Excel behaving as it is.
To be quite frank, the way that Excel implements multiline support just
doesn't cut it and is highly unproductive and barely workable for what
I'd like to do, so I really don't think we should cripple Calc to the
way Excel works.
I'd suggest that most Excel users will have "wrap text" turned on when
dealing with multilines and in this case, my proposed changes provide a
perfect xls import.
- Spreadsheets created in OpenOffice.org 2 *might* look different *if*
the result of a formula contains a newline.
Same as above for Excel import. Having a newline contained and wrap
option set in older documents should be rare. A slight difference would
be in string width, as an "inline" newline is displayed with zero width
whereas a space was, well, a space.
- Performance. The patch could be made more performant by searching for
'\n' just once in the formula result and caching the result somewhere,
see the new method ScFormulaCell::IsMultilineResult(). I'm not sure if
this is really necessary though.
See above ;-)
Everything else that I can see just works as you'd hope, for example
matrix cells as well as "Shrink to fit cell size", "Wrap text
automatically" in the Alignment tab of the Format Cells properties
dialog. This is largely because the patch simply uses the edit engine to
display the formula result if (and only if) the formula result contain
multiple lines.
I havn't yet figured out the Calc regression testing to make sure that
nothing else is broken.
Special care must be taken of places that directly or indirectly call
ScCellFormat::GetString() because those now may have to digest a newline
that they didn't have to before. Same for ScFormulaCell::GetString() and
ScEditCell::GetString(), the latter being the most invasive change of
your patch. This has to be carefully checked and might make it necessary
to introduce another method ScEditCell::GetMultilineString() to be used
in formulas and not touch ScEditCell::GetString(). Places that still
obtain a string via ScEditUtil::GetSpaceDelimitedString() now receive
a string that is different from ScEditCell::GetString(). Check for side
effects. Printing should be tested as well.
I'm aware that the change to ScEditCell::GetString is very central and
it did concern me. However, it is becoming clearer and clearer to me
that putting the fix in here is key to fixing all the other problems and
the more I've read around this area the more things I see fixed by the
patch, precisely because the fix was done in such a core piece of code.
I've found another 4 bug reports that the patch fixes and listed them in
http://www.openoffice.org/issues/show_bug.cgi?id=35913, but it needs
Eike or someone with appropriate privileges to mark the duplicates. For
those that are interested, I've attached some before and after
screenshots in #35913. I've been using the patch in anger over the last
few days and nothing seems broken, but clearly more testers are needed.
If it is easy for someone to run the tests with
the patch or point me in the right direction that would be much
appreciated.
The QA project has a bunch of tests they run on a CWS or on special
occasions. The tests reside in module qadevOOo, for Calc that would be
those in qadevOOo/tests/basic/mod/sc, the [EMAIL PROTECTED] mailing list should
be
able to provide details. Some differences in this case may be only
visually detectable though, due to different row heights and string
widths.
Okay thanks, I'll see if I can get this working once the other mods have
been put in place.
I've been developing and testing under Linux and have also
sent off my signed SCA.
Fine. Looks like the "copyright-approved" page is outdated, last
updated 2008-06-03 ...
I never heard anything back after sending emailing it in, nothing even
after chasing up. I'll wait some more.
William
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
