https://issues.apache.org/bugzilla/show_bug.cgi?id=45126





--- Comment #3 from Josh Micich <[EMAIL PROTECTED]>  2008-06-04 10:48:12 PST ---
(In reply to comment #1)
> Created an attachment (id=22069)
 --> (https://issues.apache.org/bugzilla/attachment.cgi?id=22069) [details]
> Patch
> 

I have a few suggestions regarding the proposed patch

(1) It seems like the new logic in Workbook.createBuiltInName() checks for a
built-in name clash (effectively comparing the byte field field_12_builtIn_name
on NameRecord).  If there is a clash, the new built-in name record is replaced
with a *non*-built-in name record with nameText "Excel_BuiltIn_Titles_" +
<digit> (where digit is chosen to make the name unique).  
What is the justification for creating a non-built-in name record when a
built-in name was requested?  I can't find any reference to the name
"Excel_BuiltIn_Titles".  
Perhaps the more correct operation would be to not add a name record, if one
already exists.

(2) A new junit is required, showing the mistake (as visible in latest POI
svn).

(3) The deletion of lines from the existing junit looks like it might be wrong.
 From what I can see in the API, the print-area can be set per sheet, and
therefore a workbook with more than one sheet can have multiple print areas.
The name of the unit test method and the comment also seem to support this.  I
made a modification to the test case in svn r663322 - duplicating the checks
for the multiple print areas before and after re-serialization.  This should
make POI's *current* behaviour clearer.  If POI is wrong (in attempting to
support multiple print areas) we'll need to change quite a few things. 

(4) checkNameAlreadyExists() or any method like it belongs on LinkTable.  As
mentioned above, the name check might need to take sheetIx into account for
built-in names. Should the name comparison be case-insensitive?  Either way add
a comment.


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to