Implement @skip_as_script functionality inside cell metadata

[disberd]

First attempt to implement the functionality of the @skip_as_script macro directly within the cell metadata in the notebook file.

Being able to comment out cells in the file without the need of te @skip_as_script macro is quite useful especially when developing packages with Pluto notebooks as basis.

I currently have one problem with the context menu button to toggle the skip_as_script functionality.
I tried copying the functionality from the disabled cell toggle from @lungben, but it seems the change in the metadata for the cell is only correctly registered by the front end after re-running the cell.
This is not ideal as this flag would not need re-running cell for its functionality, but I left the re-run in the added code till I get some help or understand how to toggle this without rerunning.
Maybe some feedback on this from @fonsp or @pankgeorg?

[github-actions]

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/disberd/Pluto.jl", rev="metadata-exclusive")
julia> using Pluto

[pankgeorg]

The implementation looks pretty neat, the metadata system seems quite powerful! Any idea why the tests are failing?

[disberd]

I think there was one test where a cell was specifically created programmatically without the new skip_as_script entry, but in the code to save the notebook that metadata field is explicitly accessed.
If that was the only problem, I hope my second commit fixes that

[lungben]

I tried copying the functionality from the disabled cell toggle from @lungben, but it seems the change in the metadata for the cell is only correctly registered by the front end after re-running the cell.

For cell disabling / enabling, the cell is currently always re-executed. If the cell is disabled, this does nothing, except disabling all dependent cells. If a cell is enabled, running the cell brings the notebook back into a valid state.

But for your use case I agree that the metadata should be updated without re-running the cell.

[disberd]

@fonsp I think this is now ready for review.

I did not change the visual aspect of it from the beginning as I have no nice idea on how to improve it nor much bandwidth do it in the next days.
If you prefer to avoid having the button in the context menu and you are fine with the rest of the changes, I can maybe just remove the button on the context menu and just leave this as a backend implementation that can be only accessed via the internal pluto_actions so that it's accessible to who might need it and polish the visuals in a separate PR?

[greimel]

I've just tried this and this is what happened.

1. After selecting "skip as script" the context menu doesn't disappear.
2. Removing the cell will only remove the cell visually - the defined objects are not removed

Screen.Recording.2022-06-10.at.16.20.14.mov

[greimel]

It would be really nice if skip as script automatically propagates to dependent cells.

That way one could skip using Plots and all the cells containing plots would automatically be skipped.

Or if you create some test dataset, you only need to skip the cell that defines it, to remove all your tests.

[Pangoraw]

2. Removing the cell will only remove the cell visually - the defined objects are not removed

This is another issue not related to this PR, see #2167

[disberd]

@greimel Thanks for trying this out.

I'll try out to see what is tue problem causing issue 1. (Though the idea with @fonsp was to merge the basic functionality and probably avoid having a context menu item for the moment, moving the front-end api for skipped cells to a subsequent PR)

Regarding the additional functionality it does seem interesting but I think it would need some more discussion to check whether we are not risk hitting some corner cases where you wouldn't want to skip cells automatically.
In any case, I think it's best to try and push the basic functionality as soon as possible here and then add that dependent cells skip functionality as a separate PR.

[disberd]

The failed test on the Reload from file is quite weird. It only fails on windows 1.7 and I can't understand why.
Also because it was passing before the latest commit with @Pangoraw suggestion which shouldn't have change anything related

[Pangoraw]

The tests are sometimes failing because they are timing sensitive, nothing related with the PR! 🙌

It looks great! What has been decided about styling/hiding the button for now (mentioned here) ?

[disberd]

I think @fonsp still didn't have the chance to think what to do about it. You have seen the piece of parchment TODO list during yesterday's call :D

[fonsp]

Awesome! My main changes were:

renaming to "Disable in file" and changing the wording of the hover title help

adding a popup when you click the mysterious new line next to a cell to explain what's up

With these changes, I feel like the feature (while still niche) is not too intimidating anymore! So let's just release it publicly for now, instead of adding a secret switch somewhere to enable it.

[fonsp]

[disberd]

Thanks a lot @fonsp! Definitely look nicer now!