-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added MoveFrom/MoveTo code #332
base: main
Are you sure you want to change the base?
Conversation
Learn Build status updates of commit 8aed34b: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
@microsoft-github-policy-service agree company="Microsoft" |
@twsouthwick Here is the pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds support for handling move-from and move-to revisions in Word processing documents by refactoring removal logic and introducing new helper methods.
- Refactored deletion and insertion element removal using a common RemoveElements method
- Added dedicated methods to handle insertions and move-to elements
- Updated documentation to include details about move-from and move-to elements
Reviewed Changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
samples/word/accept_all_revisions/cs/Program.cs | Refactored code to simplify element removal and added handling for move revisions |
docs/word/how-to-accept-all-revisions-in-a-word-processing-document.md | Expanded documentation with details on move-from and move-to elements |
Files not reviewed (3)
- samples/word/accept_all_revisions/cs/accept_all_revisions_cs.sln: Language not supported
- samples/word/accept_all_revisions/vb/Program.vb: Language not supported
- samples/word/accept_all_revisions/vb/accept_all_revisions_vb.sln: Language not supported
Comments suppressed due to low confidence (2)
docs/word/how-to-accept-all-revisions-in-a-word-processing-document.md:197
- Typo: 'Range Star' should be corrected to 'Range Start' in the description for moveFromRangeStart.
introduces the Move From Range Star element (moveFromRangeStart).
docs/word/how-to-accept-all-revisions-in-a-word-processing-document.md:232
- Typo: The description should refer to 'Range End' for moveToRangeEnd instead of 'Range Start'.
introduces the Move To Range Start element (moveToRangeEnd)
static void HandleInsertions(Body body, string authorName) | ||
{ | ||
// Collect all insertion elements by the specified author | ||
var insertions = body.Descendants<Inserted>().Cast<OpenXmlElement>().ToList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inserted elements are not filtered by author; consider filtering by authorName to maintain consistency with the handling of other insertion elements.
var insertions = body.Descendants<Inserted>().Cast<OpenXmlElement>().ToList(); | |
var insertions = body.Descendants<Inserted>().Where(c => c.Author?.Value == authorName).Cast<OpenXmlElement>().ToList(); |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
var moveToElements = body.Descendants<MoveToRun>().Cast<OpenXmlElement>().ToList(); | ||
moveToElements.AddRange(body.Descendants<Paragraph>() | ||
.Where(p => p.Descendants<MoveFrom>() | ||
.Any(m => m.Author?.Value == authorName))); | ||
moveToElements.AddRange(body.Descendants<MoveToRangeEnd>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var moveToElements = body.Descendants<MoveToRun>().Cast<OpenXmlElement>().ToList(); | |
moveToElements.AddRange(body.Descendants<Paragraph>() | |
.Where(p => p.Descendants<MoveFrom>() | |
.Any(m => m.Author?.Value == authorName))); | |
moveToElements.AddRange(body.Descendants<MoveToRangeEnd>()); | |
var paragraphs = body.Descendants<Paragraph>() | |
.Where(p => p.Descendants<MoveFrom>() | |
.Any(m => m.Author?.Value == authorName)); | |
var moveToRun= body.Descendants<MoveToRun>(); | |
var moveToRangeEnd = body.Descendants<MoveToRangeEnd>(); | |
var moveToElements = [.. paragraphs, .. moveToRun, .. moveToRangeEnd]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AddRange feels out of place with all the LINQ stuff - better to stay in one camp or the other
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the VB, could do the equivalent of
var moveToElements = paragraphs.Concat(moveToRun).Concat(moveToRangeEnd);
These aren't blockers, but more code cleanup that could be done later |
Learn Build status updates of commit 8aed34b: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
fixed typos and text wrapping after images
Learn Build status updates of commit 464f737: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
…/open-xml-docs into AcceptMoveFromChanges
Learn Build status updates of commit 31d79de: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
FYI @mikeebowen @tomjebo