Known Architectural Issues List

[vicchig]

I would like to keep a list of more serious issues that are not simple small bugs and require a larger redesign of the system here so that we are aware of them. I am also intending for this to prevent this same issues from being reintroduced.

Callback unsubscribe mechanism (SOLVED).

This happens when we notify() an event on the construct that also immediately needs to unsubscribe itself such as during construct deletion. This is fine if there is only one callback per type to be unsubscribed, but if there are multiple it breaks the unsubscribe for-loop.

The notify() loop runs through all callbacks of a particular type to be notified, then if we immediately call unsubscribe() as the code is notified of the event, unsubscribe() will run its own loop for removing the callbacks, but that in turn messes with the notify() loop and so not all callbacks end up getting notified even if they are not the ones being removed.

Solution 1:
The problem is not that callbacks that need to be notified are being removed. The correct callbacks are removed, it just causes errors in notifying the remaining callbacks.

Therefore, the solution in my opinion is to just remove callbacks that need to be removed AFTER all callbacks that need to be notified have been notified. I created a mechanism for this in my fix to #330. Please see the commit for the fix for how the mechanism should be used.

Related Issues:
#330
#463

Width and Height of Characters is only available for rendered chars.

Monaco methods for determining the width and height of characters within the editor only work if the current line is rendered within the editor's viewport. This causes a lot of visual issues for our highlights because they rely on the dimensions provided by monaco for the with and the height. In particular this affects:

• getOffsetForColumn()
  • Monaco documentation itself states that this only works on rendered lines.
• computeCharWidth()
  • This is a different issue as it is our internal method. It relies on lines actually having text in them to calculate the width, however, the text also seems to have a need to be rendered otherwise it returns 0.

Solution 1:
There really is none because you cannot just get the width or height of a single character. The only reason height does not break in the same manner is because it calculates the height of a line which is an HTML element that can be measured.

I calculated the quotient manually to use it in conversion between whatever unit monaco uses for font-size and px. Hopefully it is consistent.

Related Issues:
#222

EditableTxtTkn and IdentifierTkn share a lot of functionality

EditableTxtTkn and IdentifierTkn differ only slightly so I wonder if they can be merged. They both need the same behaviours. The only main difference is the regex they use for validation of the text.

Solution 1:
The fix would be to merge the two classes however at this point this requires way more work than it is worth. We have so much functionality that specifically depends on something being an IdentifierTkn that we would need to change quite a bit of our program.

Related Issues:
None and I don't expect any to appear, this is purely a design issue.

Warnings hover functionality:

Monaco's native highlight and warning message functionality do not do everything that we would want them to do and do not allow us to add onto that functionality in some non-time consuming way (it should be possible to just modify monaco's code for this).

The current hover is custom, but I would like to highlight some problems with it so that we don't repeat them.

1. Textbox-Highlight Nesting: The highlight and the textbox cannot be nested. This is because the hover requires to have a z-index of <0. Otherwise it will block pointer events for the text that lies below.
2. Event Bubbling to Text: We cannot just bubble the pointer events to the text because the highlight is not a descendant of the text in the DOM. Their common ancestor is also too far away in the tree and once reached, we would not be able to tell which text element has to have the event sent to it. Even if that was somehow possible.
3. Tying Hover to Text: There seems to be no way to identify or retrieve the text DOM node that monaco has currently focused so we cannot tie our hover to it in some way.
4. Collision Detection: The highlight area is already being used for collision detection. If the hover is in a correct position, but no hover detection is happening, it is more than likely that it is the mouseOffset variables that are not being properly updated.

Related Issues:

Warnings hover functionality:

Pyodide will not work on Safari and causes problems on Chrome.

Related Issues:

• Pyodide: Output from Python program appears as a single block after the program has run #468