BIRT-designer, advanced register: added new configuration option to control the word layout (#1969)

[speckyspooky]

• The PR include the new advanced properties for the word emitter like #1845 for PDF.
• The handling of header & footer layout is fixed and optimized

[hvbtup]

Is this breaking the API?

Probably yes. But should we really care about that?

My personal opinion here is: So what? Let me explain a bit more: I think the idea of commercial plugins for BIRT is dead anyway. The implementers you are talking about should really be contributors to the project. If they develop plugins as closed source, their companies can make a little money as a short-term effect, but in the long run neither their companies nor their customers nor the Eclipse project will benefit from it.

[merks]

If we break API but only increment the minor version then the semantic version numbers become meaningless. That's questionable.

[speckyspooky]

@wimjongman / @merks What would be the preferred way for the interface topic? I have not changed existing methods. I added only 2 new methods. If we are not able to change interfaces/APIs then we won't be able to provide new features at long term.

I can try it again to change the handling without changing the IWordWriter which needs a little bit time. Please let me know what your suggestion is.

[wimjongman]

I am not against breaking, but we must try to avoid it. And when we must do it, then we have to increment the major version number of the plugin.

Also changing any public method will break, not just interfaces. (It is only a break if the MANIFEST.MF exports the package containing the interface or the class.)

Constructor and methods

For the constructors there are ways around. e.g. instead of changing the method you can do like this (overloading):

// Old constructor calls the new constructor
Header(IPart part, Document document, int headerHeight, int headerWidth) throws IOException {
      this(part, document, headerHeight, headerWidthm, true or false);
    }

Header(IPart part, Document document, int headerHeight, int headerWidth, boolean wrapHeader) throws IOException {
        super(part);
        this.document = document;
        this.headerHeight = headerHeight;
        this.headerWidth = headerWidth;
        this.wrapHeader = wrapHeader;
    }

// in the same way the any old methods can call the new method.

Interface

The problem is that the two methods must be implemented by all that use the interface. So it is breaking. Maybe you can provide a default implementation in the interface? This way others will still use the existing functionality.

/**
 * Set the layout attribute for header and footer wrapping.
 * 
 * @param useWrappedTable use layout grid to wrap header and footer 
 */
default void setWrappedTableHeaderFooter(boolean useWrappedTable) {
    // Default implementation: does nothing or logs a message
    System.out.println("Default implementation: setWrappedTableHeaderFooter invoked with value: " + useWrappedTable);
}

https://github.com/eclipse-platform/eclipse.platform/blob/master/docs/Evolving-Java-based-APIs.md#Example-4---Adding-an-API-method

[wimjongman]

Probably yes. But should we really care about that?

Yes, we should care about it. Did you not take the oath? ;)

[speckyspooky]

I have removed the header-constructor because at our project it changed all calls. But you are right if some body use only parts of the projects it could be make trouble. So I will re-add the header constructor.

I can also add the default method but I struggle with the MANIFEST,MF change of the plugin. (I take a look into the docx-manifest but I'm not firm with the version numbers which must be changed.)

My idea is to implements an instance-check at AbstractEmitterImpl.java because the logic is only address to docx. So I will try the object and can call the method only if it is an object based on class "DocxWritter". In this case I can undo the interface change.

[speckyspooky]

@wimjongman, @merks I added the default methods to the interface. My idea wasn't working due to circle import-exports within the plugins.

Please let me know which version number I have to change at the MANIFEST.MF to fulfill your requirement of version handling for the plugin.

[merks]

FYI, I'm out of office and will not have time until next week.

[speckyspooky]

@wimjongman Please advise for the next step - addition changes necessary(?)

[speckyspooky]

I added my comments, thanks Wim!

[wimjongman]

I was just looking for a good commit message because I did not understand the commit message:

"The designer advanced register provide the configuration for the word layout (#1969)"

Next time we have to make sure to have a good commit message.

[speckyspooky]

@wimjongman I changed the commit title, hope it is more clear now.

[wimjongman]

The PR title is not super important, because we also have the issue. It is about the commit comment.

https://github.com/eclipse-birt/birt/commits/master/

Just before we do a squash and merge. We have to make sure that there is a good commit comment.