Freeze read only feature fixed for Jupyter 5.x (and compatible with Jupyter 4.x)

[tnarik]

Proposed fix for #940.

[jcb91]

Thanks for this, it looks good to me. As noted in #885 (comment), there's also a cell.metadata.deletable attribute, though it seems that non-editable cells are implicitly non-deletable, so I guess we don't need to set that as well. However, it might be good to migrate & remove the old metadata values?

[tnarik]

I wasn't sure about what you thought regarding that, but yes, a migration of the metadata would be good to have for any notebook edited with an old version of the plugin. it's also pretty straightforward and I can do that in day or even sooner.

[tnarik]

Updated the notebook to read old format ('run_control>read_only') metadata.
Also fixed an issue by which, once touched by the plugin, the 'editable' metadata was set to 'false' (because in Jupyter 5.x editable = true means the flag won't be persisted to file).

[jcb91]

Oh, I was thinking of something more like

if (cell.metadata.run_control && cell.metadata.run_control.read_only !== undefined) {
    if (cell.metadata.run_control.read_only) {
        cell.metadata.editable = false;
    }
    delete cell.metadata.run_control.read_only;
}

so that we adopt the new metadata just by loading the notebook - make sense?

[tnarik]

I just didn't want to change much the existing code, even if refactoring could make sense. Opening the notebook will immediately take care of the "upgrade" because loading the extension triggers the initialization of the cell states and the setting of the cell metadata (via `set_state`). Maybe I'm missing your point, it's the first time I touch Jupyter extension code.

[jcb91]

I just didn't want to change much the existing code, even if refactoring could make sense.

Fair enough, that's a reasonable approach 😄

Opening the notebook will immediately take care of the "upgrade" because
loading the extension triggers the initialization of the cell states and
the setting of the cell metadata (via set_state).

Yes, it will work ok, in the sense that the new metadata.editable will be set correctly, but it will also leave behind the old metadata.run_control.read_only value, which will override the metadata.editable each time the nbextension loads. I think it'd be neater to remove the old-style metadata attribute once it's been transferred to the new editable attribute. Does that make more sense?

[tnarik]

Ah, sorry. Yes, it makes perfect sense. I thought I tested that but I didn't.

[tnarik]

Created a new pull request, #968, instead of this one.