Add support for including and excluding drives from listing #89
Conversation
|
|
Co-authored-by: Afshin Taylor Darian <[email protected]>
| export async function includeDrive(driveName: string) { | ||
| await requestAPI<any>('drives/config', 'POST', { | ||
| include_drive_name: driveName | ||
| }); | ||
|
|
||
| data = { | ||
| name: driveName, | ||
| path: driveName, | ||
| last_modified: '', | ||
| created: '', | ||
| content: [], | ||
| format: 'json', | ||
| mimetype: '', | ||
| size: 0, | ||
| writable: true, | ||
| type: 'directory' | ||
| }; | ||
| return data; | ||
| } |
There was a problem hiding this comment.
| export async function includeDrive(driveName: string) { | |
| await requestAPI<any>('drives/config', 'POST', { | |
| include_drive_name: driveName | |
| }); | |
| data = { | |
| name: driveName, | |
| path: driveName, | |
| last_modified: '', | |
| created: '', | |
| content: [], | |
| format: 'json', | |
| mimetype: '', | |
| size: 0, | |
| writable: true, | |
| type: 'directory' | |
| }; | |
| return data; | |
| } | |
| export async function includeDrive(driveName: string) { | |
| await requestAPI<any>('drives/config', 'POST', { | |
| include_drive_name: driveName | |
| }); | |
| return { | |
| name: driveName, | |
| path: driveName, | |
| last_modified: '', | |
| created: '', | |
| content: [], | |
| format: 'json', | |
| mimetype: '', | |
| size: 0, | |
| writable: true, | |
| type: 'directory' | |
| }; | |
| } |
If I understand this clearly, you rely on the success/failure of the await call and if it succeeds, you don't need its output, you just return the data value ... but where is the variable data declared?
There was a problem hiding this comment.
The data object is declared here:
jupyter-drives/src/requests.ts
Lines 16 to 27 in 8089a33
It's being used for all requests to shape the data that is being sent.
There was a problem hiding this comment.
Does this answer your question?
There was a problem hiding this comment.
Oh I see, but then references might collide with each other. I don't think different API requests should be modifying a return value, though.
There was a problem hiding this comment.
Hmm, so far there were no issues with this, but it's a good point! I'll look into it and I can open up a follow-up PR to refactor the code to let each function have it's own object.
There was a problem hiding this comment.
Looking at the requests file in general, I would say just return the object literals and remove this data variable altogether throughout that file. Since these are async functions, this pattern allows one request's response to clobber another's by sharing state, and we don't need to share anything here.
There was a problem hiding this comment.
This can be a separate refactor PR if you want to merge this functionality quickly or here in this branch if you prefer.
There was a problem hiding this comment.
That's a good point! Looking over it now, as the requests then get called in contents.ts inside the Drive class where the data is used to emit the fileChanged signal, the code in requests .ts seems redundant. I can open up a follow-up PR to properly reactor the requests file to remove the data object.
Co-authored-by: Afshin Taylor Darian <[email protected]>
Add support for including and excluding specific drives from the drivebrowser listing. The functionalities are available in an integrated experience through the listed drives management dialog that can be accessed through the toolbar button.
Listed drives management dialog
Screencast.from.24.07.2025.15.36.13.webm
Remove drive command
The remove drive command is also accessible when on the context menu when right-clicking a specific drive.