Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

[[ ScriptColorization ]] Fix multiline comment colorization #4425

Open
wants to merge 4 commits into
base: develop
Choose a base branch
Loading
from

Conversation

montegoulding
Copy link
Member

I can't find a bug report on this but it's a long standing issue where the comment nesting gets messed up when you mutate the script.

@montegoulding
Copy link
Member Author

Too slow on long scripts so WIP

@montegoulding
Copy link
Member Author

This is much better than before but there's still an anomaly occasionally when returning to a script and selecting within a multiline comment it formats entered text as though the nesting is 0

@montegoulding
Copy link
Member Author

The problem appears to be when deleting or pasting a chunk of text the comment nesting deltas below the mutated range aren't updated correctly. I'm looking at shifting them up or down or possibly moving the comment delta to an IDE mode #ifdef in MCParagraph rather than try and keep track of them from the sidelines.

Once we have that right there's a a couple more things I think would be cool to add here:

  • font names and sizes for different colorization classes
  • Basic markdown support for at a minimum heading, emphasis and strong within multiline comments

@montegoulding
Copy link
Member Author

Still having trouble with this as there's a very fine line between making long scripts slow and accurately recolorizing when something can impact the entire script.

@montegoulding
Copy link
Member Author

I have a feeling what we need is to separate the mutation and the colorization and recolorize from the dirty char to the end of the field only when there's a quiet patch between text entry. This will probably resolve the issues a number of Windows users seem to have with slow script editor text entry (unbuffered keystrokes). It possibly makes the internal text replace command redundant or we could use it to do the replace in plain text so we don't get runs of the wrong color while waiting for the colorization callback. It could be that we could get away with re-colorizing in a callback only in the event there is a non-0 comment delta below the mutation and the mutation is more than one line and changed the comment delta... I'm not sure..

@runrevmark
Copy link
Contributor

It would be interesting to find out precisely what the bottleneck is on long scripts - e.g. when you add /* to the start. Part of the problem, I suspect, is the time it takes to lay things out - if there is only a change in color then it should be possible to make it relatively quick; however if there's a change in style that makes things much more problematic. Of course, if we optimized the field for monospaced fonts, and forced the S/E to use monospaced fonts then that could help a lot ;)

(Btw, I doubt it will make much difference, but the nesting code is overcomplicated in the sense that multi-line comments don't nest - so really it should just be a boolean flag for each line to indicate whether it is in a multiline comment or not).

@montegoulding
Copy link
Member Author

@runrevmark perhaps an enum for comment began, comment ended, comment continued and none?

It would be nice if we could allow a different font for comment blocks. Eventually I'd really like to have comment blocks support some basic markdown for headings, emphasis and code. So with those ideas in mind requiring monospaced fonts might not lead where I at least would like to go.

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

Successfully merging this pull request may close these issues.

2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.