[Bug 60737] New: XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 60737] New: XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

            Bug ID: 60737
           Summary: XSSFSheetXMLHandler class's inside interface
                    SheetContentsHandler should have an endSheet method
           Product: POI
           Version: 3.15-FINAL
          Hardware: PC
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: XSSF
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ---

interface SheetContentsHandler inside XSSFSheetXMLHandler class should have an
endSheet method, which should be called from the :

public void endElement(String uri, String localName, String qName)
            throws SAXException {

method's "switch" modified as follows:
...
 } else if ("sheetData".equals(localName)) {
            // Handle any "missing" cells which had comments attached
           
checkForEmptyCellComments(EmptyCellCommentsCheckType.END_OF_SHEET_DATA);
            output.endSheet();
        }
...

This is is useful when the sheet finished processing, and you need to do
something with the sheet data yet available.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

--- Comment #1 from Javen O'Neal <[hidden email]> ---
Created attachment 34758
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34758&action=edit
The patch described by comment 0

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

Javen O'Neal <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All
             Status|NEW                         |NEEDINFO

--- Comment #2 from Javen O'Neal <[hidden email]> ---
src/ooxml/java/org/apache/poi/xssf/extractor/XSSFEventBasedExcelExtractor.java
does not compile with the patch from comment 1.
The following error is given:
XSSFEventBasedExcelExtractor.SheetTextExtractor is not abstract and does not
override abstract method endSheet() in SheetContentsHandler

Could you submit a tested patch that hopefully also includes an update to the
documentation, a poi-examples file, or a unit test so that we understand how
endSheet() is used and what it's supposed to do? The title of this bug comment
0 is a bit vague on the need to have this new feature.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

--- Comment #3 from Javen O'Neal <[hidden email]> ---
(In reply to Javen O'Neal from comment #2)
> The title of this bug comment 0
correction: The title of this bug and comment 0

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

--- Comment #4 from zakim <[hidden email]> ---
Created attachment 34759
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34759&action=edit
patched XSSFEventBasedExcelExtractor

The missing file patched.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

zakim <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |NEW

--- Comment #5 from zakim <[hidden email]> ---
Attached the missing file.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

Javen O'Neal <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #6 from Javen O'Neal <[hidden email]> ---
(In reply to zakim from comment #4)
> Created attachment 34759 [details]
> patched XSSFEventBasedExcelExtractor
>
> The missing file patched.
Please submit patches generated by "svn diff", "git-svn diff" or "ant -f
patch.xml" [1]
Entire files are difficult to review and commit because other changes may be
made to that file after you attached it.

Additionally, I am unclear how this contribution will help POI. Adding a method
that does nothing may be a source of confusion for people reading or
maintaining the code. If your intent is to have POI define a method that you
plan on overriding in your own code, how do you plan on using that? Could
someone else override this inner class without modifying POI source code?
If so, perhaps this change is worthy of example code or documentation so that
others know how to override endSheet().

[1] https://poi.apache.org/guidelines.html#Submitting+Patches

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

zakim <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |NEW

--- Comment #7 from zakim <[hidden email]> ---
The file shows the changes added to the existing file, compared to poi version
3.15-FINAL as stated in the version field.
I havent checked out the entire project to send only the differences.
It was only a suggestion to make poi more usable which means in my opinion:
a more usable framework should be easy to use, with less coding.
If you need to write ten classes using the poi framework then you better write
the whole function without using poi.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

Javen O'Neal <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34759|0                           |1
        is obsolete|                            |

--- Comment #8 from Javen O'Neal <[hidden email]> ---
Created attachment 34762
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34762&action=edit
The patch corresponding to attachment 34759 against the code from 3.15-final

(In reply to zakim from comment #7)
> The file shows the changes added to the existing file, compared to poi version > 3.15-FINAL as stated in the version field.
The version field is sometimes updated when an unfixed bug is still present in
a later build, so it's better to explicitly state in a comment what the base
rev for a file is. Even better is to submit a patch, which includes the rev if
the patch was generated by Subversion.
It's probably a good idea to check out trunk anyways so that you can open the
project in Eclipse or other IDE, make your changes, and know immediately
whether there are compile-time errors with your changes.

> I haven't checked out the entire project to send only the differences.
You can perform a sparse svn checkout of POI if you don't want to wait the 10
minutes it takes to check out the trunk or one of the release tags. You could
also save the original file and modified file and use any diffing program to
generate a patch file (Linux/Mac: diff, Windows: TortoiseSVN, TortoiseGit)

> It was only a suggestion to make poi more usable which means in my opinion:
> a more usable framework should be easy to use, with less coding.
> If you need to write ten classes using the poi framework then you better
> write the whole function without using poi.
This is the part that I am confused about. How does the addition of an empty
endSheet() method make it easier for someone to use POI? Did I correctly
receive your patch file? Was there really only an addition of 3 lines?

The only other references I see to endSheet within the POI source code and
website is in BigGridDemo. Is this the inspiration for this bug?

I am genuinely interested in trying to make POI easier for people to use, but
I'm not convinced that the contributions so far accomplish that. Am I missing
some other files that have some useful behavior when the endSheet() event is
reached?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

Javen O'Neal <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

zakim <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |NEW

--- Comment #9 from zakim <[hidden email]> ---
Never mind, forget about it.
It's not the BigGridDemo, I've never seen that demo.

You can simply press the Diff button near the Attachment on the bugzilla site
and you can see the difference. I was referring to the officially relase in the
maven repo.
If you release another version with 3.15.Final name, then you have a big
problem and the diff won't help either, because your versions will be messed
up.
If I knew that a suggestion is so hard to accomplish, I would never start this.
This is a feature request I've asked for, not a bug.
I've encountered this, when I was overwriting the following interface:
XSSFSheetXMLHandler.SheetContentsHandler. I was saving the rows read from the
xlsx file by group(1 save from multiple rows) and needed to save the last group
at the end of the sheet without passing the data outside the overridden
interface.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Dominik Stadler <[hidden email]> ---
Applied via r1809371, thanks for the suggestion.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60737] XSSFSheetXMLHandler class's inside interface SheetContentsHandler should have an endSheet method

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60737

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]