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

Redraw window after delete #792

Closed
snprajwal opened this issue Mar 30, 2022 · 8 comments · Fixed by #1292
Closed

Redraw window after delete #792

snprajwal opened this issue Mar 30, 2022 · 8 comments · Fixed by #1292
Labels

Comments

@snprajwal
Copy link

Many times, after deletion, files/folders are still visible. The window should be refreshed and redrawn after the delete action is completed

@gokcehan gokcehan added the bug label Apr 2, 2022
@gokcehan
Copy link
Owner

gokcehan commented Apr 2, 2022

@snprajwal We do actually load directories after delete in here which should also trigger a redraw but I think this happened to me as well. It would help if there is a way to consistently produce this. If this is happening due to modification times not updating, there may not be much we can do about it.

@karabaja4
Copy link

karabaja4 commented Jul 18, 2022

Came here to report this bug and found this issue.

Yeah, it's very hard to reproduce it consistently, but it happens way too often not to notice.

Also, setting set period 1 doesn't seem to have any effect on directory refresh either.

@T0ille
Copy link

T0ille commented Aug 15, 2022

I also noticed that the window never redraws for me when deleting file with :delete and found this issue.

I am on r27 on WIndows. Anyone got a workaround for this?

@karabaja4
Copy link

@gokcehan

I managed to reproduce this consistently while I was working on some AUR packages (by accident).

This is my config: https://github.com/karabaja4/arch/blob/master/lf/lfrc

  1. Download the archive: https://avacyn.aerium.hr/stuff/firefox-socket-control-git.tar.gz
  2. Extract (:ex in my config) the archive by using the custom command.
  3. Move into firefox-socket-control folder.
  4. You will see a list of the following files:
firefox-socket-control
pkg
src
firefox-socket-control-git-1.5.326f892-1-any.pkg.tar.zst
firefox-socket-control-git.install
PKGBUILD
socketcontrol-1.5-fx.xpi
  1. Select the following files/folders (s in my config):
firefox-socket-control
pkg
src
firefox-socket-control-git-1.5.326f892-1-any.pkg.tar.zst
socketcontrol-1.5-fx.xpi
  1. Delete the files (d in my config)
  2. The files that should now be listed are:
firefox-socket-control-git.install
PKGBUILD

but instead, the following are (incorrectly) listed:

pkg
src
firefox-socket-control-git-1.5.326f892-1-any.pkg.tar.zst
firefox-socket-control-git.install
PKGBUILD
socketcontrol-1.5-fx.xpi

YouTube video with the above procedure shown: https://www.youtube.com/watch?v=_8jA08Ln7zk

I can reproduce it 100% of the time.

@herrbischoff
Copy link

Same issue here. On macOS 13.4 (22F66), while browsing an SMB share, lf 30 doesn't refresh the file after deleting or moving a file or folder. I have to do a manual :reload every time, which is obviously infeasible. No other file manager, including ranger, nnn, mc and even Finder exhibits this issue for me.

If there's any data I can provide you with to home in on the issue, make sure to let me know.

@joelim-work
Copy link
Collaborator

joelim-work commented Jun 8, 2023

Good news, I managed to reproduce the scenario given in #792 (comment). The issue happens on version r30 even without any custom user configuration. So the steps can be simplified to:

  1. Download the archive: https://avacyn.aerium.hr/stuff/firefox-socket-control-git.tar.gz
  2. Extract the archive using tar xvf
  3. Start lf and move into the firefox-socket-control folder
  4. Select and delete the files as specified above

After some debugging I found that when the files are deleted, the directory is loaded in two different places, which causes the following sequence of events:

  1. The selected files are deleted by calling nav.del.
  2. nav.del creates a separate thread to handle the actual file deletion.
  3. After calling nav.del, the main thread proceeds to trigger a load event.
  4. During the load event, the mtime of the directory is checked to see if it needs reloading (in this case it does).
  5. The directory is reloaded, but at this time, the thread in step 2 has only deleted some of the requested files. Any remaining files will be displayed at the end, causing the result to be inconsistent.
  6. After the directory is reloaded, the loadTime (last check time) of the directory is set to the current time, which will be slightly greater than the actual mtime of the directory.
  7. After the thread in step 2 finishes running, it triggers another load event.
  8. Similar to step 4, the mtime is checked again, but because the loadTime is now greater than the mtime due to step 6, the directory is considered to not have updated and nothing happens.
  9. The final result displayed to the user is the set of files from step 5, which represents the state of a partially completed deletion.

The above sequence of events is very complex because it involves multiple threads, hopefully the code snippets below will help to explain:

Starting point for the delete command:

lf/eval.go

Lines 1168 to 1185 in 027538e

if arg == "y" {
if err := app.nav.del(app); err != nil {
app.ui.echoerrf("delete: %s", err)
return
}
app.nav.unselect()
if gSingleMode {
app.nav.renew()
app.ui.loadFile(app, true)
} else {
if err := remote("send load"); err != nil {
app.ui.echoerrf("delete: %s", err)
return
}
}
app.ui.loadFile(app, true)
app.ui.loadFileInfo(app.nav)
}

Implementation of the nav.del function:

lf/nav.go

Lines 1451 to 1488 in 027538e

func (nav *nav) del(app *app) error {
list, err := nav.currFileOrSelections()
if err != nil {
return err
}
go func() {
echo := &callExpr{"echoerr", []string{""}, 1}
errCount := 0
nav.deleteTotalChan <- len(list)
for _, path := range list {
nav.deleteCountChan <- 1
if err := os.RemoveAll(path); err != nil {
errCount++
echo.args[0] = fmt.Sprintf("[%d] %s", errCount, err)
app.ui.exprChan <- echo
}
}
nav.deleteTotalChan <- -len(list)
if gSingleMode {
nav.renew()
app.ui.loadFile(app, true)
} else {
if err := remote("send load"); err != nil {
errCount++
echo.args[0] = fmt.Sprintf("[%d] %s", errCount, err)
app.ui.exprChan <- echo
}
}
}()
return nil
}

Part of the nav.checkDir function which handles updating the directory:

lf/nav.go

Lines 483 to 511 in 027538e

func (nav *nav) checkDir(dir *dir) {
s, err := os.Stat(dir.path)
if err != nil {
log.Printf("getting directory info: %s", err)
return
}
switch {
case s.ModTime().After(dir.loadTime):
now := time.Now()
// XXX: Linux builtin exFAT drivers are able to predict modifications in the future
// https://bugs.launchpad.net/ubuntu/+source/ubuntu-meta/+bug/1872504
if s.ModTime().After(now) {
return
}
nav.startPreview()
dir.loading = true
dir.loadTime = now
go func() {
nd := newDir(dir.path)
nd.filter = dir.filter
nd.sort()
if gOpts.dirpreviews {
nav.dirPreviewChan <- nd
}
nav.dirChan <- nd
}()


Anyway I think this issue can be solved simply by deleting the first load event, so that the only time the directory is reloaded is after the files are actually all deleted.

@karabaja4
Copy link

karabaja4 commented Jun 8, 2023

Same issue here. On macOS 13.4 (22F66), while browsing an SMB share, lf 30 doesn't refresh the file after deleting or moving a file or folder. I have to do a manual :reload every time, which is obviously infeasible. No other file manager, including ranger, nnn, mc and even Finder exhibits this issue for me.

If there's any data I can provide you with to home in on the issue, make sure to let me know.

Please note that this sounds like a completely different issue.

The issue that's being discussed here is that sometimes, very intermittently, lf doesn't update the files after a delete in a local folder, in scenarios described and solved by joelim-work above (thank you joelim!).

Files not being updated after the delete in a remote SMB folder (which sounds like an issue that can be observed 100% of the time) sounds like a different issue altogether, so it may be worth opening a new issue with a detailed description.

@herrbischoff
Copy link

Please note that this sounds like a completely different issue.

Maybe. As soon as a new release with the above changes is available, we'll see. From the issue description this race condition may actually be triggered 100% of the time on slower file systems like an SMB share.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants