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
279 stars 49 forks source link

Remove exception from wordsection #99

Closed crjc closed 1 year ago

crjc commented 1 year ago

I wasn't able to use this lib due to this exception being thrown. I've noticed a similar exception in the code base was commented out, so my quick 'fix' was to do the same here.

PrzemyslawKlys commented 1 year ago

Well I would prefer you check what type it is missing so maybe we can add a support for it? Just do the debug on it, stop, and see what type it is and add if else. This would allow us to track/and fix it. It's a temporary workaround until we're sure we support all options. the other empty if/else are basically tracked elsewhere so they don't need special treatment. I am afraid by ignoring it you may be loosing some functionality.

I should clean it up, but this is basically still very alpha product.

crjc commented 1 year ago

I've added the two types that I found were missing based on my document files. The exception might trip up on other people's documents, but I added it back anyhow.

PrzemyslawKlys commented 1 year ago

Maybe it would be better to comment it out and just add some sort of output, to log a ticket? on the other hand if it's not defined like you just did people will keep on creating ticket for types - that are not defined. I am not sure what's better.

crjc commented 1 year ago

Not sure if this is OK, but I've added a flag to WordDocument which allows for this exception to be disabled. I would have found this useful for my use case earlier.

For example:

WordDocument.ThrowNotImplementedExceptions = false;
using (WordDocument document = WordDocument.Load(filePath)) {
    ...

The exception thrown by default now has a more informative message, summarising what type hasn't been handled and encouraging the user to create an issue here.

PrzemyslawKlys commented 1 year ago

I've been thinking on it and while I believe your solution would make most sense, I am pro removing all elseif/else and just leave Header/Footer references.

If you won't interact with GutterOnRight or Bidi it will not be removed hence there's no need to throw on it at all. Anything else such as Guitter, PageMargins and so on are already added.

If someone (you?) wants support for GutterOnRight, GutterOnTop or similar

We should simply create new issue and open "support" for given functionality.

It should not throw on this because it's confusing and just not useful. Can you please remove it? I think it will make most sense? What do you think?

And if we really want to support your document type fully we should just understand what those settings are and define them, and fix them in some near future.

PrzemyslawKlys commented 1 year ago

I've opened the issues:

To track those unsupported elements

crjc commented 1 year ago

Thanks! I think that makes sense, so do you think I should remove the two if/else I added, and replace the exception with a simple Debug.WriteLine message?

PrzemyslawKlys commented 1 year ago

Ye, adding debug.writeline makes sense, so that this can be "seen" but at some point when the project will mature enough we will completely remove it.

crjc commented 1 year ago

Here we are, by the way - if this PR looks okay now, are you planning on doing a new release sometime soon?

PrzemyslawKlys commented 1 year ago

I will release today after I get back from shops.

PrzemyslawKlys commented 1 year ago

Scratch that - here you go!