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.

Sign up for GitHub

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

Jump to bottom

Fix windows delete, open file and other improvements #584

Merged
merged 11 commits into from
Feb 4, 2025
Merged

Fix windows delete, open file and other improvements #584

merged 11 commits into from
Feb 4, 2025

Conversation

lazysegtree
Copy link
Collaborator

@lazysegtree lazysegtree commented Feb 1, 2025 •
edited
Loading

Used https://github.com/hymkor/trash-go, that works in windows

Fixes - #380 - Issue in delete

Fixes - #335 - Issue in delete

Fixes - #437 - Issue in opening files

Fixes - #415 - Crash while running spf with metadata=true without exiftools installed

Adds documentation for - #514, #561 - Paste not working.

Adds documentation for - #572 - cd_on_quit not working on windows.

Added file_editor and dir_editor config options
Fixed nano being used as opening application for directory.
Fixed file editor being "" in windows ( added notepad )
Fix usage of EDITOR(env variable for opening files) while opening directories with E

Also fixes editor being empty in windows, Removed pass by value for model obj, prevent copy for panel object.

Test for windows delete

image

image

image

Also verified directory deletion

If you try to delete a directory where you lack permissions. This appears. Which is okay.
image

Successful test run in MacOS 

➜  ~/Workspace/kuknitin/superfile/testsuite git:(fix_win_delete) [10:36:40] .venv/bin/python main.py
[2025-02-03 22:36:42 -    INFO] Testcases : [CopyDirTest, CutTest, DeleteDirTest, CopyWTest, CopyTest, DeleteTest, RenameTest]
[2025-02-03 22:36:42 -    INFO] Running test CopyDirTest
[2025-02-03 22:36:44 -    INFO] Passed test CopyDirTest
[2025-02-03 22:36:44 -    INFO] Running test CutTest
[2025-02-03 22:36:46 -    INFO] Passed test CutTest
[2025-02-03 22:36:46 -    INFO] Running test DeleteDirTest
[2025-02-03 22:36:48 -    INFO] Passed test DeleteDirTest
[2025-02-03 22:36:48 -    INFO] Running test CopyWTest
[2025-02-03 22:36:49 -    INFO] Passed test CopyWTest
[2025-02-03 22:36:49 -    INFO] Running test CopyTest
[2025-02-03 22:36:51 -    INFO] Passed test CopyTest
[2025-02-03 22:36:51 -    INFO] Running test DeleteTest
[2025-02-03 22:36:53 -    INFO] Passed test DeleteTest
[2025-02-03 22:36:53 -    INFO] Running test RenameTest
[2025-02-03 22:36:55 -    INFO] Passed test RenameTest
[2025-02-03 22:36:55 -    INFO] Finished running 7 test. 7 passed
➜  ~/Workspace/kuknitin/superfile/testsuite git:(fix_win_delete) [10:36:55]

Test run in Linux 

[opc@test-client-hyd-1 testsuite]$  .venv/bin/python main.py
[2025-02-03 12:06:57 -    INFO] Testcases : [CopyDirTest, CopyTest, CutTest, RenameTest, CopyWTest, DeleteTest, DeleteDirTest]
[2025-02-03 12:06:57 -    INFO] Running test CopyDirTest
[2025-02-03 12:06:59 -    INFO] Passed test CopyDirTest
[2025-02-03 12:06:59 -    INFO] Running test CopyTest
[2025-02-03 12:07:00 -    INFO] Passed test CopyTest
[2025-02-03 12:07:00 -    INFO] Running test CutTest
[2025-02-03 12:07:02 -    INFO] Passed test CutTest
[2025-02-03 12:07:02 -    INFO] Running test RenameTest
[2025-02-03 12:07:04 -    INFO] Passed test RenameTest
[2025-02-03 12:07:04 -    INFO] Running test CopyWTest
[2025-02-03 12:07:05 -    INFO] Passed test CopyWTest
[2025-02-03 12:07:05 -    INFO] Running test DeleteTest
[2025-02-03 12:07:07 -    INFO] Passed test DeleteTest
[2025-02-03 12:07:07 -    INFO] Running test DeleteDirTest
[2025-02-03 12:07:08 -    INFO] Passed test DeleteDirTest
[2025-02-03 12:07:08 -    INFO] Finished running 7 test. 7 passed
[opc@test-client-hyd-1 testsuite]$

Test run in windows 

PS C:\Users\nitin\Documents\Programming\superfile\testsuite> .venv\Scripts\python main.py --close-wait-time 4
[2025-02-03 22:37:21 -    INFO] Testcases : [CopyWTest, CopyDirTest, CopyTest, CutTest, DeleteDirTest, DeleteTest, RenameTest]
[2025-02-03 22:37:21 -    INFO] Running test CopyWTest
[2025-02-03 22:37:27 -    INFO] Passed test CopyWTest
[2025-02-03 22:37:27 -    INFO] Running test CopyDirTest
[2025-02-03 22:37:32 -    INFO] Passed test CopyDirTest
[2025-02-03 22:37:32 -    INFO] Running test CopyTest
[2025-02-03 22:37:38 -    INFO] Passed test CopyTest
[2025-02-03 22:37:38 -    INFO] Running test CutTest
[2025-02-03 22:37:43 -    INFO] Passed test CutTest
[2025-02-03 22:37:43 -    INFO] Running test DeleteDirTest
[2025-02-03 22:37:49 -    INFO] Passed test DeleteDirTest
[2025-02-03 22:37:49 -    INFO] Running test DeleteTest
[2025-02-03 22:37:54 -    INFO] Passed test DeleteTest
[2025-02-03 22:37:54 -    INFO] Running test RenameTest
[2025-02-03 22:38:00 -    INFO] Passed test RenameTest
[2025-02-03 22:38:00 -    INFO] Finished running 7 test. 7 passed
PS C:\Users\nitin\Documents\Programming\superfile\testsuite>

Validating that pressing E (Shift+e) opened file explorer in windows, and finder in MacOS.

Validated that paste worked with ctrl+w

@lazysegtree lazysegtree requested a review from yorukot February 1, 2025 16:43
@netlify Netlify
Copy link

netlify bot commented Feb 1, 2025 •
edited
Loading

✅ Deploy Preview for superfile canceled.

Name Link
🔨 Latest commit e3e1671
🔍 Latest deploy log https://app.netlify.com/sites/superfile/deploys/67a1a3bd272d460008184c16

@lazysegtree lazysegtree changed the title fix: Used windows compatible library for trash in windows Fix windows delete, open file and other improvements Feb 1, 2025
lazysegtree
lazysegtree commented Feb 1, 2025
View reviewed changes
src/internal/handle_file_operations.go
@@ -1,10 +1,11 @@
package internal

import (
"log/slog"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dont know, but my IDE just did that automatically. Maybe there is some recommended ordering or something

lazysegtree
lazysegtree commented Feb 1, 2025
View reviewed changes
src/internal/handle_file_operations.go
@@ -18,7 +19,7 @@ import (

// Create a file in the currently focus file panel
func (m *model) panelCreateNewFile() {
panel := m.fileModel.filePanels[m.filePanelFocusIndex]
panel := &m.fileModel.filePanels[m.filePanelFocusIndex]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

panel and model are huge objects, with 10K+ bytes in size. Better to minimise their copying

lazysegtree
lazysegtree commented Feb 1, 2025
View reviewed changes
src/internal/handle_file_operations.go
@@ -481,7 +477,7 @@ func (m *model) pasteItem() {
if err != nil {
errMessage = "paste item error"
} else {
// Todo : These error cases are hard to test. We have to somehow make the paste operations fail,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My ide automatically removed trailing whitespace. Not a bad thing though

lazysegtree
lazysegtree commented Feb 1, 2025
View reviewed changes
src/internal/handle_file_operations.go
}
if editor == "" {
func (m *model) openDirectoryWithEditor() tea.Cmd {
// Todo : Move hardcoded strings to constants : "windows", and editors
Copy link
Collaborator Author

@lazysegtree lazysegtree Feb 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This fixes the wrong behaviour when we try to open directories with EDITOR - that user configure for text file.
For example if a MacOS user configured EDITOR=nano to edit text files, but if he wants to open the directory in Finder application(File explorer of MacOS), it will open the directory in nano, which is not expected.

lazysegtree
lazysegtree commented Feb 1, 2025
View reviewed changes
src/internal/handle_file_operations.go Outdated
// open is command for MacOS Finder
editor = "open"
} else {
editor = "vi"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Use vi instead of nano as default directory opener in linux. As nano cannot open directories, while using vi you can do a basic exploration through directories

@lazysegtree lazysegtree marked this pull request as draft February 3, 2025 12:10
@lazysegtree
Copy link
Collaborator Author

Will update the PR with dir_editor and file_editor option.

lazysegtree
lazysegtree commented Feb 3, 2025
View reviewed changes
website/src/content/docs/configure/custom-hotkeys.mdx
@@ -30,7 +30,7 @@ If you are a vim user, the default hotkeys may make you hate superfile.
:::

superfile default hotkeys design concept:
- All hotkeys that will change to files use `qctrl+key` (As long as you don't press ctrl your files will always be safe).
Copy link
Collaborator Author

@lazysegtree lazysegtree Feb 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed typo

@lazysegtree lazysegtree marked this pull request as ready for review February 3, 2025 16:43
@lazysegtree
Copy link
Collaborator Author

@yorukot I have made all the documentation and config changes, and updated the PR with the successful testsuite run.
Please check the PR when you are available.

@yorukot yorukot merged commit b5965dc into main Feb 4, 2025
9 checks passed
@yorukot
Copy link
Owner

yorukot commented Feb 4, 2025

LGTM thanks a lot!

@lazysegtree lazysegtree mentioned this pull request Feb 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yorukot yorukot Awaiting requested review from yorukot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lazysegtree @yorukot