Skip to content
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

assertion failure when Paragraph constructed with empty segment list #345

Closed
hwaite opened this issue Aug 2, 2016 · 9 comments
Closed
Labels

Comments

@hwaite
Copy link

hwaite commented Aug 2, 2016

org.fxmisc.richtext.model.Paragraph(PS,List<StyledText<S>>) fails when assertions are enabled and second argument is empty. Is assertion truly necessary? If so, perhaps invariant should be verified even when assertions not enabled (e.g. "if (segments.isEmpty()) throw new IllegalArgumentException();").

Stack trace:

org.fxmisc.richtext.model.Paragraph.(Paragraph.java:42)
org.fxmisc.richtext.model.Paragraph.restyle(Paragraph.java:218)
org.fxmisc.richtext.model.SimpleEditableStyledDocument.lambda$setStyleSpans$7(SimpleEditableStyledDocument.java:147)
org.fxmisc.richtext.model.SimpleEditableStyledDocument$$Lambda$1131.137893627.apply(Unknown Source:-1)
org.fxmisc.richtext.model.ReadOnlyStyledDocument.lambda$null$7(ReadOnlyStyledDocument.java:288)
org.fxmisc.richtext.model.ReadOnlyStyledDocument$$Lambda$1093.523297059.apply(Unknown Source:-1)
org.reactfx.util.Tuple2.map(Tuple2.java:31)
org.fxmisc.richtext.model.ReadOnlyStyledDocument.lambda$replace$8(ReadOnlyStyledDocument.java:287)
org.fxmisc.richtext.model.ReadOnlyStyledDocument$$Lambda$1091.1289685895.apply(Unknown Source:-1)
org.reactfx.util.Tuple2.map(Tuple2.java:31)
org.fxmisc.richtext.model.ReadOnlyStyledDocument.replace(ReadOnlyStyledDocument.java:286)
org.fxmisc.richtext.model.ReadOnlyStyledDocument.replace(ReadOnlyStyledDocument.java:276)
org.fxmisc.richtext.model.SimpleEditableStyledDocument.setStyleSpans(SimpleEditableStyledDocument.java:141)
org.fxmisc.richtext.model.StyledTextAreaModel.setStyleSpans(StyledTextAreaModel.java:566)
org.fxmisc.richtext.StyledTextArea.setStyleSpans(StyledTextArea.java:995)

@TomasMikula
Copy link
Member

Thanks for reporting! You are right, it should be an IllegalArgumentException. Probably the assertion is there from times when the constructor was not public.

@TomasMikula
Copy link
Member

Do you want to open a pull request?

@hwaite
Copy link
Author

hwaite commented Aug 2, 2016

Sure, I could fix this by throwing exception but am not 100% sure that doing so would fully address problem.

If you look at stack trace, you'll see that I'm not directly calling constructor. I make a seemingly reasonable call to setStyleSpans(int,StyleSpans) with a non-empty StyleSpans object. I probably need to investigate what's wrong with arguments to this call. Perhaps I'm adding empty span via StyleSpansBuilder? Should that be prohibited? Whatever the case, it's probable that error condition could be detected before Paragraph ctor is invoked.

@TomasMikula
Copy link
Member

Or you can provide a simple self-contained example and I will investigate when I come back from vacation.

@hwaite
Copy link
Author

hwaite commented Aug 2, 2016

Possibly caused by applying style to a line break?

public class Bug345 extends Application {
    public void start(Stage pStage) {
        pStage.setOnCloseRequest(pEvt -> Platform.exit());

        final String txt = String.join("\n", "x", "", "y");

        final CodeArea c = new CodeArea(txt);

        final VBox root = new VBox();
        root.getChildren().add(c);

        pStage.setScene(new Scene(root, 300, 250));
        pStage.show();

        final StyleSpansBuilder<Collection<String>> ssb = new StyleSpansBuilder<>();


        ssb.add(Collections.singleton("x"), 1);

        // uncomment line to prevent error
//      ssb.add(Collections.emptySet(), 1);

        ssb.add(Collections.emptySet(), 1);
        ssb.add(Collections.singleton("y"), 1);
        final StyleSpans<Collection<String>> spans = ssb.create();

        System.out.println("text len: " + txt.length());
        System.out.println("span len: " + spans.length());

        c.setStyleSpans(0, spans);
    }

    public static void main(String[] pArgs) {launch(pArgs);}
}

@TomasMikula TomasMikula added this to the 0.7 milestone Sep 16, 2016
@JordanMartinez
Copy link
Contributor

@hwaite I ran the demo you listed above and was not able to reproduce the issue.

My Environment:

Linux Mint 18.1

java version "1.8.0_121"
Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)

@hwaite
Copy link
Author

hwaite commented Mar 18, 2017

@JordanMartinez, you're running with assertions enabled?

@JordanMartinez
Copy link
Contributor

@hwaite Nope! I learned Groovy first, which has assertions enabled by default, not Java. So, that would explain why I didn't even know you had to enable them.

I can reproduce the issue now.

@JordanMartinez
Copy link
Contributor

If I update Paragraph's restyle method to return itself when middleSegs is empty, this fixes the problem. However, is this error caused by a bug in this method? Or a bug elsewhere?

public Paragraph<PS, SEG, S> restyle(int from, StyleSpans<? extends S> styleSpans) {
    int len = styleSpans.length();
    if(styleSpans.equals(getStyleSpans(from, from + len))) {
        return this;
    }

    Paragraph<PS, SEG, S> left = trim(from);
    Paragraph<PS, SEG, S> right = subSequence(from + len);

    Paragraph<PS, SEG, S> middle = subSequence(from, from + len);
    List<SEG> middleSegs = new ArrayList<>(styleSpans.getSpanCount());
    int offset = 0;
    for(StyleSpan<? extends S> span: styleSpans) {
        int end = offset + span.getLength();
        Paragraph<PS, SEG, S> text = middle.subSequence(offset, end);
        middleSegs.addAll(text.restyle(span.getStyle()).segments);
        offset = end;
    }
//// ---------------------------------
    if (middleSegs.isEmpty()) {
        return this;
    } else {
//// ---------------------------------
        Paragraph<PS, SEG, S> newMiddle = new Paragraph<>(paragraphStyle, segmentOps, middleSegs);

        return left.concat(newMiddle).concat(right);
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants