After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 60468 - Oleo import fails
Oleo import fails
Product: Gnumeric
Classification: Applications
Component: General
git master
Other All
: Low normal
: ---
Assigned To: Jody Goldberg
Jody Goldberg
Depends on:
Reported: 2001-09-13 11:01 UTC by Toralf Lund
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: ---

Oleo spreadsheet file (6.18 KB, application/x-oleo)
2001-09-13 11:02 UTC, Toralf Lund
Gnumeric version of the same file (incorrectly converted) (1.44 KB, application/x-gnumeric)
2001-09-13 11:06 UTC, Toralf Lund
Oleo screenshot (38.82 KB, image/jpeg)
2001-09-13 11:09 UTC, Toralf Lund
Proposed fix (5.23 KB, patch)
2001-09-14 11:38 UTC, Toralf Lund
none Details | Review
Proposed fix, take 2 (replaces old patch) (8.25 KB, patch)
2001-09-18 11:35 UTC, Toralf Lund
none Details | Review
Formatting updates (4.36 KB, patch)
2001-09-27 14:15 UTC, Toralf Lund
none Details | Review

Description Toralf Lund 2001-09-13 11:01:29 UTC
When trying to open an Oleo spreadsheet in gnumeric, I get

% gnumeric s16w3301.oleo 

** CRITICAL **: file cell-draw.c: line 180 (cell_draw): assertion
`cell->rendered_value' failed.

** CRITICAL **: file cell-draw.c: line 180 (cell_draw): assertion
`cell->rendered_value' failed.
[ message repeats ]

No data is visible in the normal spreadsheet window, but it does look like
*something* has been read, based on selection of individual cells. If I
save as gnumeric file, then re-open, data shows up, but is far from correct.

Example files will be attached...
Comment 1 Toralf Lund 2001-09-13 11:02:54 UTC
Created attachment 5597 [details]
Oleo spreadsheet file
Comment 2 Toralf Lund 2001-09-13 11:06:25 UTC
Created attachment 5598 [details]
Gnumeric version of the same file (incorrectly converted)
Comment 3 Toralf Lund 2001-09-13 11:09:02 UTC
Created attachment 5599 [details]
Oleo screenshot
Comment 4 Toralf Lund 2001-09-13 11:12:47 UTC
Oh, and I was wondering if the correct converter was being used, but
Plugin list marks "Gnu Oleo" with "[in memory]" after the file is
opened. That does mean that the plugin has been used, right?
Comment 5 Jody Goldberg 2001-09-13 19:05:07 UTC
The warnings about unrendered cells were fixed several versions ago (the
current release is 0.70).  However, the quality (or lack thereover) of the
oleo importer has not changed.  It has not been maintained and could use some
serious work.
Comment 6 Toralf Lund 2001-09-14 09:05:06 UTC
No, I definitely get these error messages with the version I checked
out from CVS yesterday.

As for the import code, I think someone here will try to update it.
Comment 7 Toralf Lund 2001-09-14 11:38:09 UTC
Created attachment 5604 [details] [review]
Proposed fix
Comment 8 Toralf Lund 2001-09-14 11:51:58 UTC
The way I see it, there are two major problems with the current oleo

1. Formatting commands are ignored completely. That's a bad idea even
if we aren't interested in the formating as such, since these lines
may update the "active" row or column no (i.e. the row/column
subsequent lines that don't specify it themselves apply to.)
2. No attempt is made to parse formulas.

The patch I just attached addresses both these issues. It adds code
that will pick up row or column no from formatting lines, and also
introduces a formula parser. The routine doesn't fully interprete the
expressions, though, it just replaces Oleo cell specs with Gnumeric
equivalents (relative cell ids are currently converted to absolute
ones, but that could probably be changed quite eaily), then calls
Gnumeric's own parser. This approach works for simple cases at least.
Comment 9 Toralf Lund 2001-09-14 14:33:30 UTC
I've now also trying to handle the actual format strings properly, but
I haven't been very succesful so far. What I'm doing is

	char fmt_string[100];
	Range range;
	MStyle *mstyle = mstyle_new_default ();

	  range.start.col=range.end.col=OLEO_TO_GNUMERIC (col);

	  range.start.row=range.end.row=OLEO_TO_GNUMERIC (row);

        <build fmt_string>
        mstyle_set_align_h(mstyle, <correct alignment>); 

        if (fmt_string[0]) {
          mstyle_set_format_text (mstyle, fmt_string);
	sheet_style_set_range(sheet, &range, mstyle);
is that the correct approach? What I want is to build an mstyle
structure (I think) then somehow apply it to a cell _or_ an entire row
or column; row or col is negative if the appropriate value was left
out from the file (I'm assuming that means the formatting applies to
the entire column or row.)  
Comment 10 Toralf Lund 2001-09-14 14:45:51 UTC
The cell->rendered_value assertion message goes away if I call
cell_render_value(), or cell_get_rendered_text() etc., after inserting
data with cell_set_value()/cell_set_expr().
Comment 11 Jody Goldberg 2001-09-14 15:03:34 UTC
You should not have to call the render routines at that level.  I'll have a
look at your patch and see if I can replicate your render problem shortly.  In
the mean time, this conversation would probably go faster if you could drop by
#gnumeric @

thanks for looking at this.
Comment 12 Toralf Lund 2001-09-18 11:32:48 UTC
Hmmm. I'm not really an IRC kind of person, but we'll see...

In any case, I've now prepared a new version of my patch. Changes
since the last time:
1. Started adding support for "formatting" commands. None
   of it works yet, unfortunately.
2. Better code for references in formulas. CellRef type
   taken into use, which made the code both simpler and
   more general, and allowed the distinction between
   relative and absolute references to be made. 
   (Actually, all references were relative earlier, and
   not absolute like I said.)
3. cell_set_expr_and_value() called when there is an
   expression. cell_set_expr() after cell_set_value()
   like the previous version did seemed to clear
   the value.
4. Workaround for the "rendered_value" assertion
   problem, see above.
5. cell_queue_recalc() called after inserting value
   and/or expression. Whether or not we want this is
   a matter of taste; the main intention is to make sure
   error value is displayed right away when a formula
   couldn't be parsed. It may also mean that inserting
   expression _and_ value, see 3, is meaningless.
Comment 13 Toralf Lund 2001-09-18 11:35:02 UTC
Created attachment 5617 [details] [review]
Proposed fix, take 2 (replaces old patch)
Comment 14 Jody Goldberg 2001-09-18 19:56:35 UTC
There is no need to call cell_render or to handle recalcs at this level.
Adding a call to

        sheet_flag_recompute_spans (sheet);

after the sheet is created is sufficent.  The rest of the patch certainly
improves things, although it needs to be reformatted to match the rest of
Gnumeric.  I'll apply it and make the necessary tweaks, but I'll leave the bug
open until the formatting works more smoothly.
Comment 15 Toralf Lund 2001-09-26 14:01:04 UTC
Seems like I won't have time to look at the formatting stuff today,
either. I will get around to it eventually, though, so if in the
meantime anyone could add a brief outline of what calls I should use
to set the value type and font, justification etc. of a cell, row or
the sheet, I would be very grateful.
Comment 16 Toralf Lund 2001-09-27 14:15:29 UTC
Created attachment 5714 [details] [review]
Formatting updates
Comment 17 Toralf Lund 2001-09-27 14:27:00 UTC
It turns out I was using the correct calls all along, they just didn't
have any effect because the range was incorrectly initialised.
Changing types of row/col arguments to oleo_set_format() was enought
to get some of the formatting right.

However, due to Oleo's "incremental" approach to cell specification,
setting format for ranges of cells just won't do, we need to keep
track of the current format, and apply it to each cell as values are
inserted. The latest patch will make the appropriate modifications to
the code.

There is no proper support for "default" entries yet, though; we may
want to update style of a range when parsing these, and merge them
somehow with "cell" style set up later.

Also, please revise calls to mstyle_ref() and mstyle_unref() as I'm
not eentirely sure how these are meant to be used.
Comment 18 Jody Goldberg 2001-12-07 05:45:20 UTC
Patch has finally been applied.
Comment 19 Andreas J. Guelzow 2001-12-07 07:05:21 UTC
Does this mean that this bug should now be closed ?
Comment 20 Jody Goldberg 2001-12-07 14:00:10 UTC
Good point.  I suppose we can reopen if more development is done.