Menu

#581 TCoolEdit renders whitespace as operators

8
pending
CoolPrj (46)
1
2024-10-18
2024-10-15
No

There was a bug regarding the rendering of whitespace in the syntax highlighting parsers: COLORINDEX_OPERATOR was used instead of COLORINDEX_WHITESPACE.

This issue has now been fixed [r7148].

The screenshot below shows the issue (bug on the right, fix on the left):

Whitespace rendering bug screenshot

Related

Bugs: #582
Commit: [r7148]
Discussion: CoolEdit in C++Builder 10.x
Feature Requests: #252
Wiki: OWLNext_Roadmap_and_Prereleases

Discussion

  • Vidar Hasfjord

    Vidar Hasfjord - 2024-10-17

    Note that the fix [r7148] merely corrects the syntax category, hence the issue with overproduction of blocks remains.

     else{
    
    -  DEFINE_BLOCK (nPos, COLORINDEX_OPERATOR);
    +  DEFINE_BLOCK (nPos, _istspace(pszChars[nPos]) ? COLORINDEX_WHITESPACE : COLORINDEX_OPERATOR);
       bRedefineBlock = true;
       bDecIndex = true;
     }
    

    I implemented the fix this way, because it is focused and low-risk, unlikely to create a regression.

    However, a more comprehensive, yet riskier, fix would skip consecutive whitespace characters:

    else if (_istspace(pszChars[nPos]))
    {
      DEFINE_BLOCK(nPos, COLORINDEX_WHITESPACE);
      while (I < len && _istspace(pszChars[I]))
        ++I;
      bRedefineBlock = true;
      bDecIndex = true;
    }
    else{
      DEFINE_BLOCK (nPos, COLORINDEX_OPERATOR);
      bRedefineBlock = true;
      bDecIndex = true;
    }
    

    In my limited testing with the C++ parser, this better fix seems to work well, and it reduces the block count substantially.

    Note that nPos is a temporary variable that might be equal to the character index I or one less, depending on the state of bDecIndex at the top of the for-loop, so I made sure the whitespace skipping would work in both cases.

    Regarding those boolean variables, I don't fully understand how they are supposed to work, and whether they are (both) needed here or not. I just kept the code as before, since removing them didn't quite work, and I wasn't prepared to review and rewrite the whole code.

    Let me know if you see any issues with this better fix. Try it out. If it works well for you in thorough testing, I might commit it.

    Otherwise, I'll leave this here just as a suggestion for improvement.

     

    Related

    Commit: [r7148]

  • Vidar Hasfjord

    Vidar Hasfjord - 2024-10-18

    The optimised solution (scanning the whitespace in one go and producing only one TTextBlock for each span) was implemented in the overhaul of TCSyntaxParser [r7154][feature-requests:#252]. The new implementation produces very few superfluous blocks, if any.

     

    Related

    Commit: [r7154]
    Feature Requests: #252

  • Sebastian Ledesma

    HI Vidar:

    I just ended reviewing the optimized fix and it looked fine for me.
    I specially like the idea to have a better and faster OWLMaker/CoolEdit editor.

     
    👍
    1
  • Vidar Hasfjord

    Vidar Hasfjord - 2024-10-18

    Thanks Sebastian, for reviewing the code! If you hadn't been willing to do so, I wouldn't have committed it. Hopefully, it hasn't created (too many) regressions. I shudder to think that OWLMaker may now crash just by viewing a file due to a bug in my eager refactorisation. Well — no risk, no reward. Test well, and test again! :-)

     

Log in to post a comment.

MongoDB Logo MongoDB