-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
Remove exception from wordsection #99
Conversation
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. |
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. |
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. |
… disabling exception if necessary.
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:
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. |
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. |
I've opened the issues:
To track those unsupported elements |
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 |
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. |
Here we are, by the way - if this PR looks okay now, are you planning on doing a new release sometime soon? |
I will release today after I get back from shops. |
Scratch that - here you go! |
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.