Created attachment 5597 [details]
Oleo spreadsheet file

Created attachment 5598 [details]
Gnumeric version of the same file (incorrectly converted)

Created attachment 5599 [details]
Oleo screenshot

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?

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.

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.

Created attachment 5604 [details] [review]
Proposed fix

The way I see it, there are two major problems with the current oleo
plugins:

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.

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 ();
	
	mstyle_ref(mstyle);
	range_init_full_sheet(&range);

	if(col>=0)
	  range.start.col=range.end.col=OLEO_TO_GNUMERIC (col);

	if(row>=0)
	  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);
	
	mstyle_unref(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.)  

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().

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 @ irc.gnome.org.

thanks for looking at this.

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.

Created attachment 5617 [details] [review]
Proposed fix, take 2 (replaces old patch)

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.

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.

Created attachment 5714 [details] [review]
Formatting updates

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.

Patch has finally been applied.

Does this mean that this bug should now be closed ?

Good point.  I suppose we can reopen if more development is done.