EvotecIT / OfficeIMO

Fast and easy to use cross-platform .NET library that creates or modifies Microsoft Word (DocX) and later also Excel (XLSX) files without installing any software. Library is based on Open XML SDK
MIT License
261 stars 47 forks source link

Potential Bug: Does iteration over TableCell's Paragraphs property allow mutation? #213

Closed tmheath closed 4 months ago

tmheath commented 4 months ago

public List<WordParagraph> Paragraphs => WordSection.ConvertParagraphsToWordParagraphs(_document, _tableCell.ChildElements.OfType<Paragraph>()); ConvertParagraphToWordParagraphs has a lot in it, I'm looking for a particular problem trying to figure everything out about it because as yet I haven't found anything. I see this property here, is the result only for accessing or does it also allow modification of the true Paragraphs underlying? I'm trying to delete all paragraphs in a table cell using this property for iteration right now is why I'm asking, should I be trying something completely different from this for that?

PrzemyslawKlys commented 4 months ago

You should only get paragraphs from it, but then once you have that paragraph you can also delete it. Each table cell has paragraphs and you can remove one or all of them except the first one. If you delete the first one in each table cell you need to add at least one back or word document will not be usable.

Each Paragraph has Remove() method. One could probably improve this with other methods allowing RemoveAll, but so far what is there seems to wortk?

internal static void Example_BasicTables1(string folderPath, bool openWord) {
    Console.WriteLine("[*] Creating standard document with tables");
    string filePath = System.IO.Path.Combine(folderPath, "Document with Tables1.docx");
    using (WordDocument document = WordDocument.Create(filePath)) {
        var paragraph = document.AddParagraph("Basic paragraph - Page 4");
        paragraph.ParagraphAlignment = JustificationValues.Center;

        document.AddParagraph();

        WordTable wordTable = document.AddTable(3, 4, WordTableStyle.PlainTable1);
        wordTable.Rows[0].Cells[0].Paragraphs[0].Text = "Test 1";
        wordTable.Rows[1].Cells[0].Paragraphs[0].Text = "Test 2";
        wordTable.Rows[2].Cells[0].Paragraphs[0].Text = "Test 3";
        // align to center
        wordTable.Rows[2].Cells[3].Paragraphs[0].Text = "Center";
        wordTable.Rows[2].Cells[3].Paragraphs[0].ParagraphAlignment = JustificationValues.Center;

        // align to right
        wordTable.Rows[1].Cells[3].Paragraphs[0].Text = "Right";
        wordTable.Rows[1].Cells[3].Paragraphs[0].ParagraphAlignment = JustificationValues.Right;

        // align it on paragraph outside of table
        var paragraph1 = wordTable.Rows[0].Cells[0].Paragraphs[0].AddParagraph();
        paragraph1 = paragraph1.AddParagraph();
        paragraph1.AddText("Ok");
        paragraph1.ParagraphAlignment = JustificationValues.Center;

        var paragraph2 = wordTable.Rows[1].Cells[0].Paragraphs[0].AddParagraphAfterSelf();
        paragraph2 = paragraph2.AddParagraphAfterSelf();
        paragraph2.AddText("Ok2");

        var paragraphBefore = wordTable.Rows[1].Cells[0].Paragraphs[0].AddParagraphBeforeSelf();
        paragraphBefore = paragraphBefore.AddParagraphBeforeSelf();
        paragraphBefore.AddText("Ok but Before");

        wordTable.Rows[2].Cells[0].Paragraphs[0].AddParagraphAfterSelf().AddParagraphAfterSelf().AddParagraphAfterSelf().Text = "Works differently";

        int paragraphCount = wordTable.Rows[1].Cells[0].Paragraphs.Count;
        Console.WriteLine(wordTable.Rows[1].Cells[0].Paragraphs.Count); // should be 5
        for (int i = 1; i < paragraphCount; i++) {
            wordTable.Rows[1].Cells[0].Paragraphs[0].Remove();
        }
        Console.WriteLine(wordTable.Rows[1].Cells[0].Paragraphs.Count); // should be 1

        Console.WriteLine(wordTable.Style);

        // lets overwrite style
        wordTable.Style = WordTableStyle.GridTable6ColorfulAccent1;

        document.Save(openWord);
    }
}
tmheath commented 4 months ago

So calling Clear on that property doesn't delete all paragraphs (I know a paragraph is required)?

Trying to pin something down and figure out exactly what is going on between your code and mine.

As far as this issue is concerned though, got it

PrzemyslawKlys commented 4 months ago

This is a readonly list. Whatever methods are available directly on the list are "builtin" and not attached to this project. You have to use methods that are done on the classes directly. I think its possible to write some code so on clear on list or similar it would do its job, but thats not done.

tmheath commented 4 months ago

` and FillWordContainer (container: WordContainer) (content: DocumentPart list) =

for part in content do

    match container with

    | Cell cell ->
        let addpar (text: string) = cell.AddParagraph(text, true)
        addPart part addpar cell.AddTable
        cell.Paragraphs[0].Remove()
        cell.Paragraphs[0].Remove()
    | Doc document ->
        addPart part document.AddParagraph document.AddTable

` A DocumentPart is either a Cell of OfficeIMO.Word.TableCell or a Doc of OfficeIMO.Word.WordDocument. The content is being added correctly to the document. Removing the first paragraph of every cell, and the second as I added it to do, is having no effect (as is passing both true and false to cell.AddParagraph(text)). I got the example running, and played around a bit, I'm going to look into if this is some kind of weird interop thing. If there's something obvious in that snippet that I'm doing wrong to cause that, please point it out.

Thanks, I'll let you know if I find anything

Apparently F# formats weird by default... I don't know how to get it better

tmheath commented 4 months ago

Is there any reason for the first Paragraph in a table cell to be special? The other question, are you even trying to have F# as a target?

I'm having a hard time thinking about what might be different.

PrzemyslawKlys commented 4 months ago

TableCell has to have at least one paragraph. If you remove it, Word will not open the word document. So it's not special. You can remove all paragraphs, but you need to add new one back. Otherwise it will fail.

tmheath commented 4 months ago

My next step is going to set up that same example script you sent, but in an F# environment and see if it works, even if that doesn't explain the answer directly it will still be easier to play around in, I feel as if I've tried everything that I can.

tmheath commented 4 months ago

I can now confirm, the behavior is different between F# and C# for some reason, at least on my version of the runtime here at work. I have translated that example into fsx script in exactly the same way. If you intend on supporting F# then I will dig in and see if I can figure this out, if you don't see it worth bothering over then that's fine, but I'm not sure what I'm going to do (honestly probably translate what I've already got). The translated script, it's output, and it's stack trace from my environment below in that order.

#r "DocumentFormat.OpenXml.dll"

#r "OfficeIMO.Word.dll"

// For more information see https://aka.ms/fsharp-console-apps

let folderPath = "."

let openword = false

printfn "[*] Creating standard document with tables"

let filepath = System.IO.Path.Combine (folderPath, "Document with Tables1.docx")

let document = OfficeIMO.Word.WordDocument.Create(filepath)

let paragraph = document.AddParagraph "Basic paragraph - Page 4"

paragraph.ParagraphAlignment <- DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Center

document.AddParagraph()

let wordTable = document.AddTable (3, 4, OfficeIMO.Word.WordTableStyle.PlainTable1)

wordTable.Rows[0].Cells[0].Paragraphs[0].Text = "Test 1"

wordTable.Rows[1].Cells[0].Paragraphs[0].Text = "Test 2"

wordTable.Rows[2].Cells[0].Paragraphs[0].Text = "Test 3"

wordTable.Rows[2].Cells[3].Paragraphs[0].Text = "Center"

wordTable.Rows[2].Cells[3].Paragraphs[0].ParagraphAlignment <- 

DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Center

wordTable.Rows[1].Cells[3].Paragraphs[0].Text = "Right"

wordTable.Rows[1].Cells[3].Paragraphs[0].ParagraphAlignment <- 

DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Right

let mutable paragraph1 = wordTable.Rows[0].Cells[0].Paragraphs[0].AddParagraph()

paragraph1 = paragraph1.AddParagraph()

paragraph1.AddText "Ok"

paragraph1.ParagraphAlignment <- DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Center

let mutable paragraph2 = wordTable.Rows[1].Cells[0].Paragraphs[0].AddParagraphAfterSelf()

paragraph2 = paragraph2.AddParagraphAfterSelf()

paragraph2.AddText "Ok2"

let mutable paragraphBefore = wordTable.Rows[1].Cells[0].Paragraphs[0].AddParagraphBeforeSelf()

paragraphBefore = paragraphBefore.AddParagraphBeforeSelf()

paragraphBefore.AddText "Ok but Before"

wordTable.Rows[2].Cells[0].Paragraphs[0].AddParagraphAfterSelf().AddParagraphAfterSelf().AddParagraphAfterSelf().Text <- 
"Works differently"

printfn "%i" wordTable.Rows[1].Cells[0].Paragraphs.Count // Should be five

for _ = 1 to 4 do

    printfn "Hi"

    wordTable.Rows[1].Cells[0].Paragraphs[0].Remove()

printfn "%i" wordTable.Rows[1].Cells[0].Paragraphs.Count // Should be one

printf "%s" (wordTable.Style.ToString())

wordTable.Style <- OfficeIMO.Word.WordTableStyle.GridTable6ColorfulAccent1

document.Save(openword)
[*] Creating standard document with tables

5

Hi

Hi

Hi

Hi

1

PlainTable1
System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at System.Collections.Generic.List`1.get_Item(Int32 index)
   at <StartupCode$FSI_0001>.$FSI_0001.main@() in c:\Users\Tim.Heath\Desktop\F#sample\Program.fsx:line 33
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
Stopped due to error

tbh, didn't think this was supposed to be a thing. If you want to support F#, I can go ahead and translate each example from C# to F# in a pr if you like.

tmheath commented 4 months ago

For debugging, I added the HI print.... You probably should ignore that

PrzemyslawKlys commented 4 months ago

at which point the error is thrown?

tmheath commented 4 months ago

False alarm

I was using the built in code runner on the VSCode extension, running fsi directly, the code does not throw any exceptions. Sorry about that.

I'm comparing the output now. I think it was being run twice.

tmheath commented 4 months ago

image The table on the right is C#, left is F# the code used to generate is similar (F# being the script above, C# being the example you showed).

If you want to support C# then I'll dig into this, I can also go ahead and translate your example files if you like, it's not trouble.

tmheath commented 4 months ago

My guess is mutability, and I have no idea how to fix without digging.

tmheath commented 4 months ago

I'm going to try wrapping the calls to remove and the other methods/property assignments in something to chain them together so that I only use the very last reference, I'm not sure that is a workaround or not though.

PrzemyslawKlys commented 4 months ago

I don't know F# enough but code below gives exact values as mine in C#. Using <- instead of = seems to be standard for F#

let folderPath = "."

let openword = false

printfn "[*] Creating standard document with tables"

let filepath = System.IO.Path.Combine (folderPath, "Document with Tables1.docx")

let document = OfficeIMO.Word.WordDocument.Create(filepath)

let paragraph = document.AddParagraph "Basic paragraph - Page 4"

paragraph.ParagraphAlignment <- DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Center

document.AddParagraph() |> ignore

let wordTable = document.AddTable (3, 4, OfficeIMO.Word.WordTableStyle.PlainTable1)

wordTable.Rows[0].Cells[0].Paragraphs[0].Text <- "Test 1"

wordTable.Rows[1].Cells[0].Paragraphs[0].Text <- "Test 2"

wordTable.Rows[2].Cells[0].Paragraphs[0].Text <- "Test 3"

wordTable.Rows[2].Cells[3].Paragraphs[0].Text <- "Center"

wordTable.Rows[2].Cells[3].Paragraphs[0].ParagraphAlignment <- DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Center

wordTable.Rows[1].Cells[3].Paragraphs[0].Text <- "Right"

wordTable.Rows[1].Cells[3].Paragraphs[0].ParagraphAlignment <- DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Right

let mutable paragraph1 = wordTable.Rows[0].Cells[0].Paragraphs[0].AddParagraph()

paragraph1 <- paragraph1.AddParagraph()

paragraph1.AddText("Ok") |> ignore

paragraph1.ParagraphAlignment <- DocumentFormat.OpenXml.Wordprocessing.JustificationValues.Center

let mutable paragraph2 = wordTable.Rows[1].Cells[0].Paragraphs[0].AddParagraphAfterSelf()

paragraph2 <- paragraph2.AddParagraphAfterSelf()

paragraph2.AddText("Ok2") |> ignore

let mutable paragraphBefore = wordTable.Rows[1].Cells[0].Paragraphs[0].AddParagraphBeforeSelf()

paragraphBefore <- paragraphBefore.AddParagraphBeforeSelf()

paragraphBefore.AddText "Ok but Before" |> ignore

wordTable.Rows[2].Cells[0].Paragraphs[0].AddParagraphAfterSelf().AddParagraphAfterSelf().AddParagraphAfterSelf().Text <- "Works differently"

printfn "%i" wordTable.Rows[1].Cells[0].Paragraphs.Count // Should be five

for _ = 1 to 4 do

    printfn "Hi"

    wordTable.Rows[1].Cells[0].Paragraphs[0].Remove()

printfn "%i" wordTable.Rows[1].Cells[0].Paragraphs.Count // Should be one

printf "%s" (wordTable.Style.ToString())

wordTable.Style <- OfficeIMO.Word.WordTableStyle.GridTable6ColorfulAccent1

document.Save(true)

Seeing as i get warning when using equals I would assume it's usage is different in F#, then in C#

Severity    Code    Description Project File    Line    Suppression State   Details
Warning FS0020  The result of this equality expression has type 'bool' and is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'.  ConsoleApp1 C:\Support\GitHub\OfficeIMO.F\ConsoleApp1\Program.fs    25  Active  
tmheath commented 4 months ago

You are correct, = is comparison of a property while <- is assignment.

tmheath commented 4 months ago

I see, that should have been obvious to me as I wrote that, I'll check real quick to make sure it's not platform dependent but I expect it to work, will re-open if it doesn't.

Thanks and apologies for that. Would those translations be welcome if this works?

PrzemyslawKlys commented 4 months ago

You mean like creating OfficeIMO.ExamplesFsharp or how do you want to do it?

tmheath commented 4 months ago

Yeah it works... It's not a matter of platform difference.

Honestly, just looked it over again, probably the best way would be to have OfficeIMO.Examples/C#/ and F#/ be copies, but that does involve much more than word alone (What I'm using at work for). I'm open to that but it's going to go slower, but will cover everything. Alternatively I can create a copy of each file, renamed to .fsx so that it's just a script file to show the differences. I'm going to be doing those from home and not work, if you have any opinion on how, then it's up to you, otherwise I'd probably just copy the root folder and try about translating everything.

PrzemyslawKlys commented 4 months ago

So I guess in the end I will want to create some kind of website with examples and references / docs for this project. This make sense to add it there then. Simply submit examples / blogs. Translating everything I am not sure makes sense. After all you could probably take all features of OfficeIMO and create 1-3 examples that cover all settings in one big document.

I don't want you to spend time translating something, where I've added a lot of examples simply because I was testing how it works, and just left it there.

tmheath commented 4 months ago

Gotcha alright... Will do, thanks