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

Improve readonly editor behavior #13403

Merged
merged 2 commits into from
Feb 21, 2024
Merged

Improve readonly editor behavior #13403

merged 2 commits into from
Feb 21, 2024

Conversation

msujew
Copy link
Member

@msujew msujew commented Feb 20, 2024

What it does

Improves our readonly handling for monaco editors (editors can now dynamically change being readonly and writable).

Additionally adds a readonly mode to notebook editors. This new readonly mode disables a lot of notebook features:

  • Editing (code & markdown)
  • Execution
  • Drag & drop
  • Creating new cells

This change lays the groundwork for a few features in the collaboration feature (where readonly can be toggled) and the changes required for #13353.

How to test

  1. Open a monaco editor (or notebook editor).
  2. Run the command Toggle File System Readonly. It toggles whether the file URI scheme is treated as readonly.
  3. Play around with the editors. You should not be able to manipulate the editors.
  4. Open a new editor. It should open in readonly mode.
  5. Run the command again. Everything should work as normal.

Review checklist

Reminder for reviewers

@msujew msujew added editor issues related to the editor monaco issues related to monaco notebook issues related to notebooks labels Feb 20, 2024
Copy link
Contributor

@jonah-iden jonah-iden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this intended to not work for new not yet saved files? it makes sense since they are not yet in the filesystem. I just want to make sure

@jonah-iden
Copy link
Contributor

Also seems like there is an regression. When creating new markdown cells you get following error in the console and its not possible to edit them anymore. Even when readonly is disabled

Uncaught Error Error: MISSING extHostDocument for notebook cell: vscode-notebook-cell:/c%3A/Typefox/Open_Source/theia_test_workspace/notebooks/test-notebook1.ipynb#W2sZmlsZQ%3D%3D%20
    at deserialize (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\rpc-message-encoder.ts:184:106)
    at read (c:\Typefox\Open_Source\theia\node_modules\msgpackr\unpack.js:424:1)
    at readObject (c:\Typefox\Open_Source\theia\node_modules\msgpackr\unpack.js:522:1)
    at recordDefinition (c:\Typefox\Open_Source\theia\node_modules\msgpackr\unpack.js:992:1)
    at read (c:\Typefox\Open_Source\theia\node_modules\msgpackr\unpack.js:418:1)
    at checkedRead (c:\Typefox\Open_Source\theia\node_modules\msgpackr\unpack.js:197:1)
    at unpack (c:\Typefox\Open_Source\theia\node_modules\msgpackr\unpack.js:104:1)
    at decode (c:\Typefox\Open_Source\theia\node_modules\msgpackr\unpack.js:176:1)
    at decode (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\rpc-message-encoder.ts:163:29)
    at parse (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\rpc-message-encoder.ts:167:21)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\rpc-protocol.ts:88:93)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:170:42)
    at invoke (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:178:26)
    at fire (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:321:36)
    at handleData (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\channel.ts:255:38)
    at handleMessage (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\channel.ts:209:29)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\channel.ts:167:61)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:170:42)
    at invoke (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:178:26)
    at fire (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:321:36)
    at handleMessages (c:\Typefox\Open_Source\theia\packages\plugin-ext\src\common\rpc-protocol.ts:255:39)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\plugin-ext\src\common\rpc-protocol.ts:199:49)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:170:42)
    at invoke (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:178:26)
    at fire (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:321:36)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\plugin-ext\src\hosted\browser\hosted-plugin.ts:369:42)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:170:42)
    at invoke (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:178:26)
    at fire (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:321:36)
    at postMessage (c:\Typefox\Open_Source\theia\packages\plugin-ext\src\hosted\browser\hosted-plugin-watcher.ts:36:32)
    at onRequest (c:\Typefox\Open_Source\theia\packages\core\src\common\messaging\proxy-factory.ts:166:49)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\messaging\proxy-factory.ts:145:80)
    at handleRequest (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\rpc-protocol.ts:232:39)
    at handleMessage (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\rpc-protocol.ts:104:26)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\rpc-protocol.ts:88:66)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:170:42)
    at invoke (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:178:26)
    at fire (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:321:36)
    at handleData (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\channel.ts:255:38)
    at handleMessage (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\channel.ts:209:29)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\message-rpc\channel.ts:167:61)
    at <anonymous> (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:170:42)
    at invoke (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:178:26)
    at fire (c:\Typefox\Open_Source\theia\packages\core\src\common\event.ts:321:36)
    at messageHandler (c:\Typefox\Open_Source\theia\packages\core\src\browser\messaging\ws-connection-source.ts:156:54)
    at Emitter.emit (c:\Typefox\Open_Source\theia\node_modules\@socket.io\component-emitter\index.mjs:136:1)
    at emitEvent (c:\Typefox\Open_Source\theia\node_modules\socket.io-client\build\cjs\socket.js:519:1)
    at onevent (c:\Typefox\Open_Source\theia\node_modules\socket.io-client\build\cjs\socket.js:506:1)
    at onpacket (c:\Typefox\Open_Source\theia\node_modules\socket.io-client\build\cjs\socket.js:474:1)
    at Emitter.emit (c:\Typefox\Open_Source\theia\node_modules\@socket.io\component-emitter\index.mjs:136:1)
    at <anonymous> (c:\Typefox\Open_Source\theia\node_modules\socket.io-client\build\cjs\manager.js:237:1)

@msujew
Copy link
Member Author

msujew commented Feb 21, 2024

Also seems like there is an regression. When creating new markdown cells you get following error in the console and its not possible to edit them anymore. Even when readonly is disabled

The error is unrelated to the issue, but I fixed both. The error logged was actually due to a race condition in the notebook plugin frontend.

@msujew
Copy link
Member Author

msujew commented Feb 21, 2024

is this intended to not work for new not yet saved files? it makes sense since they are not yet in the filesystem. I just want to make sure

Yes, they use the untitled uri scheme, so they're unaffected by the command that toggles the readonly editors. It only toggles the readonly mode for files with the file scheme. Not necessarily intended, but in line with expectations.

Copy link
Contributor

@jonah-iden jonah-iden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the race condition. Code looks good to me so approved

This was referenced Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor issues related to the editor monaco issues related to monaco notebook issues related to notebooks
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants