Fixed #890

[oscargus]

This should fix #890

• Change in CHANGELOG.md described?
• Changes in pull request outlined? (What, why, ...)
• Tests created for changes? Not relevant
• Tests green?

[simonharrer]

👍

[tobiasdiez]

I think the method

public String doLayout(BibEntry bibtex, BibDatabase database) {
        return doLayout(bibtex, database, null);
    }

in layout.java should be changed too.

And then there is

@Test
    public void testHighlightingInvalidParameter() {
        String content = "Test Word Content";

        TextArea ta = new TextArea("", content);

        //should not matter at all
        ta.highlightPattern(null);
    }

which also sets the highlightPattern to null...thus I would expect (i.e. add test for it) that an NPE is thrown.

Otherwise 👍

[oscargus]

I think that is the method I changed. Is there another method (I searched, but rather quickly)?

Yes, the layout package could benefit from more testing...

[tobiasdiez]

No you are right...you exactly changed the method I mentioned. My bad 😄.
What I meant in the second part is that setting the higlightPattern to null does not throw an exception (since the test pass) although I would have expected one. So probably a Objects.requireNonNull is missing somewhere.

[oscargus]

But there is a corresponding method in LayoutEntry, which I think should be fixed as well.

Ah, OK! I'll have a look (or maybe the highlighter is never invoked in the test case?).

[oscargus]

I fixed the other place as well, but happened to change to more use of switching... In the long term it would make sense to change the integers now used to an enumeration.

I also realized that all \n are stripped from the fields when doing a layout, so even though e.g. HTMLChars will convert the line breaks to something clever, they are not there anymore once the formatter is invoked... Not sure where they go missing though...

[tobiasdiez]

👍