eclipse-platform / eclipse.platform.ui

Eclipse Platform
https://projects.eclipse.org/projects/eclipse.platform
Eclipse Public License 2.0
79 stars 168 forks source link

Undesirable behavior at org.eclipse.jface.text.rules.DefaultDamagerRepairer #1999

Open VladimirIorio opened 4 months ago

VladimirIorio commented 4 months ago

Let's make sure issue is not already fixed in latest builds first.

Steps to reproduce

My bug is related to the behavior of the method createPresentation at the mentioned class org.eclipse.jface.text.rules.DefaultDamagerRepairer. I copied part of the code of that method below, to explain better:

        while (true) {
            IToken token= fScanner.nextToken();
            if (token.isEOF())
                break;

            TextAttribute attribute= getTokenTextAttribute(token);
            if (lastAttribute != null && lastAttribute.equals(attribute)) {
                length += fScanner.getTokenLength();
                firstToken= false;
            } else {
                if (!firstToken)
                    addRange(presentation, lastStart, length, lastAttribute);
                firstToken= false;
                lastToken= token;
                lastAttribute= attribute;
                lastStart= fScanner.getTokenOffset();
                length= fScanner.getTokenLength();
            }
        }

The test if (lastAttribute != null && lastAttribute.equals(attribute)) verifies whether the last attribute is equal to the current attribute. In this case, a range is not added, only the length is updated to build a longer token using the same attribute. This strategy allows for reducing the number of ranges when subsequent tokes have the same attribute. But this behavior is undesirable in the case described below.

Suppose I have a document with the following text: Token1 TokenToBeSkipped Token2 Suppose TokenToBeSkipped is a part of the text that is simply skipped by my scanner, returning no token. Suppose that I want to assign the SAME attribute to Token1 and Token2 but I will not process TokenToBeSkipped , leaving that part of the text with the standard attribute of the document.

In the case above, the method createPresentation will process Token1 and Token2 in the sequence. As the two tokens have the same attribute, the current length will be updated using the length of Token2, not considering that the text may have several characters between Token1 and Token2. The result is assigning the current attribute to Token1 and part of the token that was skipped by the scanner.

I was able to make my project work as desired by making MyScanner.nextToken return a "dummy" Token.UNDEFINED token with length 0, between Token1 and Token2 , so createPresentation will not receive two tokens with the same attribute in the sequence. But the code is unnecessarily complicated, having to test if two subsequent tokens have the same attribute.

I understand that the code of createPresentation may reduce the number of ranges in the case described. But I believe the code should work also for an example as described above.

Suggestion for fixing the code: test ALSO whether the offset of the current token is immediately after the end of the previous token, indicating that parts of the document were not skipped between the two tokens. In this case, the current length could be updated.

Community

jukzi commented 4 months ago

@VladimirIorio the bug title and description are not specific enough. Please clearly state a minimal reproducer with actual and expected behaviour.