-
-
Notifications
You must be signed in to change notification settings - Fork 90
library: Remove the need for noop code files #689
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
Conversation
This reverts commit 6514c98.
| if (language === "JavaScript") { | ||
| await previewer.update(true); | ||
|
|
||
| // We have to create a new file each time | ||
| // because gjs doesn't appear to use etag for module caching | ||
| // ?foo=Date.now() also does not work as expected | ||
| // TODO: File a bug | ||
| const [file_javascript] = Gio.File.new_tmp("workbench-XXXXXX.js"); | ||
| await file_javascript.replace_contents_async( | ||
| new GLib.Bytes(document_javascript.code_view.buffer.text || " "), | ||
| null, | ||
| false, | ||
| Gio.FileCreateFlags.NONE, | ||
| null, | ||
| ); | ||
| let exports; | ||
| try { | ||
| exports = await import(`file://${file_javascript.get_path()}`); | ||
| } catch (err) { | ||
| await previewer.update(true); | ||
| throw err; | ||
| } finally { | ||
| file_javascript | ||
| .delete_async(GLib.PRIORITY_DEFAULT, null) | ||
| .catch(console.error); | ||
| } | ||
| previewer.setSymbols(exports); | ||
| } else if (language === "Vala") { | ||
| if (!isValaEnabled()) { | ||
| action_extensions.activate(null); | ||
| return; | ||
| } | ||
|
|
||
| compiler_vala = compiler_vala || ValaCompiler({ session }); | ||
| const success = await compiler_vala.compile(); | ||
| if (success) { | ||
| await previewer.useExternal(); | ||
| if (await compiler_vala.run()) { | ||
| await previewer.open(); | ||
| } else { | ||
| await previewer.useInternal(); | ||
| } | ||
| } | ||
| } else if (language === "Rust") { | ||
| if (!isRustEnabled()) { | ||
| action_extensions.activate(null); | ||
| return; | ||
| } | ||
|
|
||
| compiler_rust = compiler_rust || RustCompiler({ session }); | ||
| const success = await compiler_rust.compile(); | ||
| if (success) { | ||
| await previewer.useExternal(); | ||
| if (await compiler_rust.run()) { | ||
| await previewer.open(); | ||
| } else { | ||
| await previewer.useInternal(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was extracted into the compile function
src/window.js
Outdated
| const buffer = langs[language.toLowerCase()].document.code_view.buffer; | ||
| // Do nothing if there is no code to avoid compile errors | ||
| if (buffer.text.trim() === "") { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only addition / change to the compile logic
|
I am fine with the changed behavior. I didn't test the branch locally yet, though. |
|
@Hofer-Julian I'm confident it works well. I'd like to hear your thoughts on
|
| const language = getLanguage("javascript"); | ||
| const language_tag = this.#createLanguageTag(language); | ||
| this._languages_box.append(language_tag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was necessary because demos without Code didn't have a .js file (but they had Rust and Vala files that did nothing)
With this we don't show language for demos without code anymore
I would mind if the code panel can be used, just for users to notice that it doesn't do anything. |
|
It is hidden by default (via |
|
The branch doesn't seem to work as expected for me Bildschirmaufzeichnung.vom.2023-10-17.17-37-10.webm |
|
@Hofer-Julian thanks for catching, I forgot to remove the placeholder which was inadequate because it would also show in user saved projects when switching lang. Done. You might still get older files in the build dir, try again after clean build. I also added an override to open the demo with JavaScript if it's not available in Rust or Vala. Updated the description. |
Thanks, can try it again in the evening. |
Hofer-Julian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected
Noop code files were introduced in #579
So that "Run" on demos with node code in Vala or Rust would work.
This is an alternative solution that
The downside is that if the user opens the "Code" panel on these, there isn't much to extend from. But it's also an issue with the noop files and since we have multi-windows, they can check an example with "JavaScript", "Rust" or "Vala".