Skip to content

Conversation

@donglining
Copy link

read trackChange in sdtcontent

@pjfanning
Copy link
Member

pjfanning commented May 16, 2025

Thanks but can you add tests? Have you run the existing tests? Can you provide some background about the change?

@pjfanning
Copy link
Member

-1 from me

I think the change tracking should not be hacked as if is a text run. If we were to add such changes, I think they should not be ignored by the parser by default and the change data should be stored in dedicated classes. The last thing we need is to have our POI Word parser returning deleted paragraphs and text as if they were still visible in the doc.

@donglining
Copy link
Author

donglining commented May 17, 2025

Thanks but can you add tests? Have you run the existing tests? Can you provide some background about the change?

Currently, the change tracking in XWPFSDTContent cannot be read. I can add some tests
I believe change tracking should be readable, allowing users to determine whether to display it. In XWPFParagraph, insertion (ins) change tracking is currently read as run text.

like this , the 'EF' ins trackChange can‘t read ,

image

@pjfanning
Copy link
Member

-1 from me

I think the change tracking should not be hacked as if is a text run. If we were to add such changes, I think they should not be ignored by the parser by default and the change data should be stored in dedicated classes. The last thing we need is to have our POI Word parser returning deleted paragraphs and text as if they were still visible in the doc.

I will try try to veto this change. The change tracking should not be treated like a text run. It needs it own dedicated handling.

@donglining
Copy link
Author

-1 from me
I think the change tracking should not be hacked as if is a text run. If we were to add such changes, I think they should not be ignored by the parser by default and the change data should be stored in dedicated classes. The last thing we need is to have our POI Word parser returning deleted paragraphs and text as if they were still visible in the doc.

I will try try to veto this change. The change tracking should not be treated like a text run. It needs it own dedicated handling.

like XWPFParagraph , change tracking treated like a text run.
image

@pjfanning
Copy link
Member

If you add test coverage then this can be reviewed but it is bad practice to provide PRs with no description and no tests.

@donglining
Copy link
Author

If you add test coverage then this can be reviewed but it is bad practice to provide PRs with no description and no tests.

Got it. I'll proceed with adding test cases.

addNewLine = false;
XWPFRun xRun = (XWPFRun) o;
// don't include the text if reviewing is enabled and this is a deleted run
if (xRun.getCTR().getDelTextArray().length == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use isEmpty() instead of length == 0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make the corresponding changes here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but Here, it's an array, not a collection.

@pjfanning
Copy link
Member

I downloaded the docx in the PR. Word shows the suggested changes but they are just suggestions. They are not the saved state of the document.

@donglining
Copy link
Author

I downloaded the docx in the PR. Word shows the suggested changes but they are just suggestions. They are not the saved state of the document.

Yes, but the INS (insertion) track changes in XWPFParagraph are being read as TextRun objects. I want them to have consistent behavior (i.e., either all treated as tracked changes or all as final text).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants