Fixes renaming files from title while using a custom drive #12849

hbcarlos · 2022-07-24T19:24:40Z

Fixes #12755
cc @jtpio @martinRenou

References

Code changes

User-facing changes

Backwards-incompatible changes

Yes, the rename dialog now receives the context instead of the path.

jupyterlab-probot · 2022-07-24T19:24:42Z

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

martinRenou · 2022-07-25T07:48:14Z

Thanks! If I understand correctly it should close #12755 ?

packages/docregistry/src/context.ts

jtpio · 2022-07-27T10:18:21Z

Thanks @hbcarlos for working on this.

Just tried on Binder after installing the jupyterlab-filesystem-access extension (https://github.com/jupyterlab-contrib/jupyterlab-filesystem-access) and there seems to be some cases where the new name is not propagated (to the file browser or the main area widget title):

rename-drive.mp4

Not sure yet but this might also be related to the extension itself.

hbcarlos · 2022-07-27T10:38:52Z

Thanks, @jtpio! Let me check what is going on with the update of the title.

Not sure yet but this might also be related to the extension itself.

Yes, I believe when renaming from the file browser, the title is not updated because the file browser doesn't emit the signal fileChanged (or something similar).

But I'm not sure what is going on when renaming from the main menu. I need to check that.

hbcarlos · 2022-07-31T11:29:31Z

@jtpio I just tried locally, and if you wait a bit, the file gets renamed.
The remaining issue comes from the jupyterlab-filesystem-access extensions. As you can see here:

jupyterlab/packages/services/src/contents/index.ts

Lines 1189 to 1212 in 8066d22

    
           async rename( 
        
             oldLocalPath: string, 
        
             newLocalPath: string 
        
           ): Promise<Contents.IModel> { 
        
             const settings = this.serverSettings; 
        
             const url = this._getUrl(oldLocalPath); 
        
             const init = { 
        
               method: 'PATCH', 
        
               body: JSON.stringify({ path: newLocalPath }) 
        
             }; 
        
             const response = await ServerConnection.makeRequest(url, init, settings); 
        
             if (response.status !== 200) { 
        
               const err = await ServerConnection.ResponseError.create(response); 
        
               throw err; 
        
             } 
        
             const data = await response.json(); 
        
             validate.validateContentsModel(data); 
        
             this._fileChanged.emit({ 
        
               type: 'rename', 
        
               oldValue: { path: oldLocalPath }, 
        
               newValue: data 
        
             }); 
        
             return data; 
        
           }

the default drive from lab, emits a signal after renaming the file but that's not the case with the extension, see:
https://github.com/jupyterlab-contrib/jupyterlab-filesystem-access/blob/3c7d326a8de6b79c7f506d02ead949bd54eca384/src/drive.ts#L211-L221
and the issue we are seeing in your screencast is that the file browser doesn't update the list of files until it fetches them.

We can merge this PR and fix the remaining issues in the extension.

jtpio · 2022-08-02T12:34:21Z

Version 0.5.2 of the jupyterlab-filesystem-access extension was just released: https://github.com/jupyterlab-contrib/jupyterlab-filesystem-access/releases/tag/v0.5.2

It includes your fix in jupyterlab-contrib/jupyterlab-filesystem-access#41 @hbcarlos, thanks!

jtpio · 2022-08-02T12:46:47Z

Version 0.5.2 of the jupyterlab-filesystem-access extension was just released: https://github.com/jupyterlab-contrib/jupyterlab-filesystem-access/releases/tag/v0.5.2

Will have a second look on Binder with this new version.

jtpio · 2022-08-02T13:15:55Z

Arf Binder seems to be broken (looks unrelated to this PR):

jtpio · 2022-08-02T13:27:27Z

This is likely because of the ypy-websocket issue:

ImportError: cannot import name 'YDoc' from 'ypy_websocket.websocket_server'

for more information, see https://pre-commit.ci

Fix typo Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>

jtpio · 2022-08-02T13:28:27Z

Rebasing to make sure we grab the latest state of master.

jtpio · 2022-08-02T14:48:20Z

Looking better after the rebase. Just checked on Binder and it seems to be behaving correctly:

rename-drive-2.mp4

jtpio · 2022-08-02T15:03:51Z

Yes, the rename dialog now receives the context instead of the path.

Just pushed a commit to add this change to https://jupyterlab.readthedocs.io/en/latest/extension/extension_migration.html#jupyterlab-3-x-to-4-x.

hbcarlos · 2022-08-03T12:18:18Z

Thanks, @jtpio!

jtpio

Thanks!

github-actions bot added pkg:docmanager pkg:docregistry labels Jul 24, 2022

github-actions bot assigned hbcarlos Jul 24, 2022

hbcarlos requested review from jtpio and martinRenou July 24, 2022 19:25

hbcarlos added bug api-change A change that should be accompanied by a major version increase labels Jul 24, 2022

fcollonval added this to the 4.0.0 milestone Jul 25, 2022

jtpio reviewed Jul 27, 2022

View reviewed changes

packages/docregistry/src/context.ts Outdated Show resolved Hide resolved

hbcarlos marked this pull request as draft July 27, 2022 10:38

hbcarlos marked this pull request as ready for review July 31, 2022 11:21

hbcarlos mentioned this pull request Jul 31, 2022

Emit signal after renaming a file jupyterlab-contrib/jupyterlab-filesystem-access#40

Closed

hbcarlos and others added 4 commits August 2, 2022 15:28

Fixes renaming files from title while using a custom drive

e40b2c1

[pre-commit.ci] auto fixes from pre-commit.com hooks

50dfb25

for more information, see https://pre-commit.ci

lint

f900bab

Update packages/docregistry/src/context.ts

80da56c

Fix typo Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>

jtpio force-pushed the rename_title branch from 4a0d84f to 80da56c Compare August 2, 2022 13:28

Update extension migration guide

3dc14c2

github-actions bot added the documentation label Aug 2, 2022

jtpio approved these changes Aug 3, 2022

View reviewed changes

jtpio merged commit 7b57518 into jupyterlab:master Aug 3, 2022

hbcarlos deleted the rename_title branch August 3, 2022 19:23

jtpio mentioned this pull request Sep 30, 2022

Update to Lumino 2 and JupyterLab 4.0.0a29 jupyter/notebook#6539

Merged

19 tasks

qqdaiyu55 mentioned this pull request Jan 3, 2023

Service worker fixes, JupyterLab 3.5.1 jupyterlite/jupyterlite#899

Merged

15 tasks

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2023

Uh oh!

Fixes renaming files from title while using a custom drive #12849

Fixes renaming files from title while using a custom drive #12849

Uh oh!

Conversation

hbcarlos commented Jul 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

References

Code changes

User-facing changes

Backwards-incompatible changes

Uh oh!

jupyterlab-probot bot commented Jul 24, 2022

Uh oh!

martinRenou commented Jul 25, 2022

Uh oh!

Uh oh!

jtpio commented Jul 27, 2022

Uh oh!

hbcarlos commented Jul 27, 2022

Uh oh!

hbcarlos commented Jul 31, 2022

Uh oh!

jtpio commented Aug 2, 2022

Uh oh!

jtpio commented Aug 2, 2022

Uh oh!

jtpio commented Aug 2, 2022

Uh oh!

jtpio commented Aug 2, 2022

Uh oh!

jtpio commented Aug 2, 2022

Uh oh!

jtpio commented Aug 2, 2022

Uh oh!

jtpio commented Aug 2, 2022

Uh oh!

hbcarlos commented Aug 3, 2022

Uh oh!

jtpio left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hbcarlos commented Jul 24, 2022 •

edited

Loading