Skip to content
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

SFTP Feature: implement Create Folder #6049

Merged
merged 8 commits into from Mar 30, 2022
Merged

SFTP Feature: implement Create Folder #6049

merged 8 commits into from Mar 30, 2022

Conversation

boonkerz
Copy link
Contributor

@boonkerz boonkerz commented Mar 29, 2022

first try to make sftp better
hope my coding skills are enouth :)

Fixes #4945

Copy link
Owner

@Eugeny Eugeny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great! I've added a few comments, and I think the PR would also benefit greatly by adding a "New directory" button to the toolbar, next to "Upload" :)

@@ -12,6 +12,7 @@ import type { FileEntry, Stats } from 'ssh2-streams'
export interface SFTPFile {
name: string
fullPath: string
directory: string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems unnecessary, (it's just posixPath.dirname(fullPath))

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so... is this field really needed?

tabby-ssh/src/sftpContextMenu.ts Outdated Show resolved Hide resolved
@boonkerz boonkerz requested a review from Eugeny March 30, 2022 12:13
@boonkerz
Copy link
Contributor Author

does tabby have an alert system to show an notification like "directory successfully created" or "directory could not be created" ?

const modal = this.ngbModal.open(SFTPCreateDirectoryModalComponent)
const directoryName = await modal.result
if (directoryName !== '') {
this.sftp.mkdir(path.join(this.path, directoryName)).finally(() => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be then (or better, just await) to prevent nagivating into a folder that failed to get created

@@ -12,6 +12,7 @@ import type { FileEntry, Stats } from 'ssh2-streams'
export interface SFTPFile {
name: string
fullPath: string
directory: string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so... is this field really needed?

@Eugeny
Copy link
Owner

Eugeny commented Mar 30, 2022

does tabby have an alert system to show an notification like "directory successfully created" or "directory could not be created" ?

Yep! You can inject the NotificationsService: https://docs.tabby.sh/classes/NotificationsService.html

Documentation for tabby-core

const savedPath = this.path
if (this.path === savedPath) {
await this.navigate(this.path)
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've correctly noticed that this is wrong! I've just fixed the statement order in the master branch - you'll need to merge it back into your branch.

@boonkerz
Copy link
Contributor Author

boonkerz commented Mar 30, 2022

does tabby have an alert system to show an notification like "directory successfully created" or "directory could not be created" ?

Yep! You can inject the NotificationsService: https://docs.tabby.sh/classes/NotificationsService.html

**NotificationsService | tabby-core**Documentation for tabby-core

only info,notice and error are available i have added success or should i use info?

image

Documentation for tabby-core

@Eugeny
Copy link
Owner

Eugeny commented Mar 30, 2022

Just use notice for this one.

@boonkerz
Copy link
Contributor Author

i have switcht to then and catch for directory creation and remove directory in sftpfile

@Eugeny
Copy link
Owner

Eugeny commented Mar 30, 2022

Perfect 🚀

@Eugeny Eugeny merged commit 89bbd66 into Eugeny:master Mar 30, 2022
@Issues-translate-bot
Copy link
Collaborator

The translator bot has detected that this issue body's language is not English, and has translated it automatically.


Perfect 🚀

@boonkerz boonkerz deleted the sftp branch March 30, 2022 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add right click option to SFTP to create new folder/empty file/...
3 participants