Add an ability to query files in the current directory as displayed in lf #1949
Conversation
There was a problem hiding this comment.
The way query works by design is that the gState.data is only evaluated when running a shell command (e.g. !lf -remote "query $id files"). Otherwise if you try to keep the gState.data up to date at all times as you've done here, it becomes very messy and error prone - when making changes in the future, it's very easy to write code that will change the internal state but then forget to update gState.data as well.
This does mean that there is extra overhead whenever running a shell command, but I haven't noticed any slowdown in performance.
So for the record I managed to get it working myself, something like below:
diff --git a/app.go b/app.go
index eaa913c..e56cae0 100644
--- a/app.go
+++ b/app.go
@@ -552,6 +552,7 @@ func (app *app) runShell(s string, args []string, prefix string) {
gState.data["cmds"] = listCmds().String()
gState.data["jumps"] = listJumps(app.nav.jumpList, app.nav.jumpListInd).String()
gState.data["history"] = listHistory(app.cmdHistory).String()
+ gState.data["files"] = listFiles(app.nav.currDir()).String()
gState.mutex.Unlock()
cmd := shellCommand(s, args)
diff --git a/ui.go b/ui.go
index 54dd103..67c48a9 100644
--- a/ui.go
+++ b/ui.go
@@ -1204,6 +1204,16 @@ func listHistory(history []cmdItem) *bytes.Buffer {
return b
}
+func listFiles(dir *dir) *strings.Builder {
+ b := new(strings.Builder)
+
+ for _, file := range dir.files {
+ fmt.Fprintln(b, file.path)
+ }
+
+ return b
+}
+
func listMarks(marks map[string]string) *bytes.Buffer {
t := new(tabwriter.Writer)
b := new(bytes.Buffer)
|
Thanks for the feedback! Yes, I understand. Your implementation is the first thing I tried after looking at how the other After some testing, I see that it panics in cmd on-cd &{{
echo noop
}}
# this causes the panic
on-cdDetails
So at it should include the same guards my export has. E.g. like this
diff --git a/app.go b/app.go
index eaa913c..a383bbb 100644
--- a/app.go
+++ b/app.go
@@ -552,6 +552,8 @@ func (app *app) runShell(s string, args []string, prefix string) {
gState.data["cmds"] = listCmds().String()
gState.data["jumps"] = listJumps(app.nav.jumpList, app.nav.jumpListInd).String()
gState.data["history"] = listHistory(app.cmdHistory).String()
+ // gState.data["files"] = listFiles(app.nav.currDir()).String()
+ app.nav.exportFilesInCurrDir()
gState.mutex.Unlock()
cmd := shellCommand(s, args)
diff --git a/nav.go b/nav.go
index 0935ff0..5baffac 100644
--- a/nav.go
+++ b/nav.go
@@ -718,6 +718,18 @@ func (nav *nav) exportFiles() {
}
}
+// Updates gState.data["files"], doesn't lock
+func (nav *nav) exportFilesInCurrDir() {
+ if !nav.init {
+ return
+ }
+ dir := nav.currDir()
+ if dir.loading {
+ return
+ }
+ gState.data["files"] = listFiles(dir).String()
+}
+
func (nav *nav) dirPreviewLoop(ui *ui) {
var prevPath string
for dir := range nav.dirPreviewChan {
diff --git a/ui.go b/ui.go
index 54dd103..67c48a9 100644
--- a/ui.go
+++ b/ui.go
@@ -1204,6 +1204,16 @@ func listHistory(history []cmdItem) *bytes.Buffer {
return b
}
+func listFiles(dir *dir) *strings.Builder {
+ b := new(strings.Builder)
+
+ for _, file := range dir.files {
+ fmt.Fprintln(b, file.path)
+ }
+
+ return b
+}
+
func listMarks(marks map[string]string) *bytes.Buffer {
t := new(tabwriter.Writer)
b := new(bytes.Buffer)
I don't remember precisely where else it fails, but I remember there was something, which is why I went with backtracking from The basic testing shows that it works, including the filter which I initially suspected would fail. I'll see if I can spot anything else. In regards to the added latency, it seems fine.
Details
diff --git a/app.go b/app.go
index eaa913c..9657948 100644
--- a/app.go
+++ b/app.go
@@ -541,6 +541,7 @@ func (app *app) runCmdSync(cmd *exec.Cmd, pause_after bool) {
// ! Yes No Yes Yes Yes Pause and then resume
// & No Yes No No No Do nothing
func (app *app) runShell(s string, args []string, prefix string) {
+ startRS := time.Now()
app.nav.exportFiles()
app.ui.exportSizes()
app.ui.exportMode()
@@ -552,6 +553,10 @@ func (app *app) runShell(s string, args []string, prefix string) {
gState.data["cmds"] = listCmds().String()
gState.data["jumps"] = listJumps(app.nav.jumpList, app.nav.jumpListInd).String()
gState.data["history"] = listHistory(app.cmdHistory).String()
+ startFiles := time.Now()
+ app.nav.exportFilesInCurrDir()
+ log.Printf("exportFilesInCurrDir() took %s", time.Since(startFiles))
+ log.Printf("all runShell() exports took %s", time.Since(startRS))
gState.mutex.Unlock()
cmd := shellCommand(s, args)
diff --git a/nav.go b/nav.go
index 0935ff0..5baffac 100644
--- a/nav.go
+++ b/nav.go
@@ -718,6 +718,18 @@ func (nav *nav) exportFiles() {
}
}
+// Updates gState.data["files"], doesn't lock
+func (nav *nav) exportFilesInCurrDir() {
+ if !nav.init {
+ return
+ }
+ dir := nav.currDir()
+ if dir.loading {
+ return
+ }
+ gState.data["files"] = listFiles(dir).String()
+}
+
func (nav *nav) dirPreviewLoop(ui *ui) {
var prevPath string
for dir := range nav.dirPreviewChan {
diff --git a/ui.go b/ui.go
index 54dd103..67c48a9 100644
--- a/ui.go
+++ b/ui.go
@@ -1204,6 +1204,16 @@ func listHistory(history []cmdItem) *bytes.Buffer {
return b
}
+func listFiles(dir *dir) *strings.Builder {
+ b := new(strings.Builder)
+
+ for _, file := range dir.files {
+ fmt.Fprintln(b, file.path)
+ }
+
+ return b
+}
+
func listMarks(marks map[string]string) *bytes.Buffer {
t := new(tabwriter.Writer)
b := new(bytes.Buffer)
|
|
@MahouShoujoMivutilde Good catch with finding the panic, I forgot that functions like func listFiles(nav *nav) string {
// return "" if there's an error
}Also just out of interest, what are the implications if the directory is |
presented in lf (including filter/sorting) `lf -remote "query $id files"` This version updates `files` list only before running shell commands.
MahouShoujoMivutilde
force-pushed
the
query-files
branch
from
15d099d to
33896aa
Compare
|
I can't find what else was wrong with the export in Maybe that's all it was, just various custom So, I switch implementations in favor of yours. A bug I discovered is that with #!/usr/bin/env bash
# previewer script
echo "Started slow preview demo..."
sleep 5s
echo "Done!"
This is not the case with (and to be fair, my initial implementation has the same problem, too) Looking at the places where I don't understand why this is needed at all Ideally, If it's a file under cursor - we don't care, because we already display its name and user can interact with it. And the preview script shouldn't prevent interaction with files. Preview is just a nice to have thing, but not at all necessary for renaming/deleting etc. And if it's a directory - why should we treat it differently? It doesn't matter if the preview is done. You already can do anything you'd want with it, and if you care about the preview - you'll just wait for it. Looking at blame #842 comes up. It sets the current dir to loading when running the previewer. I don't understand why because the file previewer seems to do fine without it...? And it's not like the content/sorting of the current directory should be changed. Why doesn't it just use file previewer logic? Like, yes, in this implementation, you can't just comment out and expect the dir previewer script to display fine. Makes sense. But why was it even implemented like that? Other than that loading shouldn't be a problem. I think it's lesser of two evils to return empty, because then user sees it immediately instead of "wait, I know this file should be here" some time after opening the external app - and it would just be pressing while true; do touch /tmp/lf-test/$RANDOM.blob ; done
while true; do touch /tmp/lf-test ; done
and 32k files, it's really hard trigger. |
|
@MahouShoujoMivutilde I don't use the As for the definition of
The purpose of 1) is for performing some initialization on a The purpose of 2) is to simply display Originally before Another weird thing I found with the #!/bin/sh
printf "[%s]: previewing %s: start\n" "$(date)" "$1" >> /tmp/lf.log
sleep 1
printf "[%s]: previewing %s: end\n" "$(date)" "$1" >> /tmp/lf.logI get the following output: This presumably also means that a directory won't be marked as loaded unless all the directories before it have as well, which is counterintuitive as directories should be loaded independently of each other. Although in practice it probably doesn't matter much since the EDIT: I hacked around with the existing code a bit and managed to get Diff (click to expand)diff --git a/app.go b/app.go
index 5c246bd..e6df3c2 100644
--- a/app.go
+++ b/app.go
@@ -258,7 +258,6 @@ func (app *app) writeHistory() error {
// separate goroutines and sent here for update.
func (app *app) loop() {
go app.nav.previewLoop(app.ui)
- go app.nav.dirPreviewLoop(app.ui)
var serverChan <-chan expr
if !gSingleMode {
@@ -336,7 +335,6 @@ func (app *app) loop() {
app.quit()
app.nav.previewChan <- ""
- app.nav.dirPreviewChan <- nil
log.Print("bye!")
@@ -510,7 +508,6 @@ func (app *app) loop() {
func (app *app) runCmdSync(cmd *exec.Cmd, pause_after bool) {
app.nav.previewChan <- ""
- app.nav.dirPreviewChan <- nil
if err := app.ui.suspend(); err != nil {
log.Printf("suspend: %s", err)
diff --git a/nav.go b/nav.go
index 0935ff0..d275329 100644
--- a/nav.go
+++ b/nav.go
@@ -175,7 +175,6 @@ type dir struct {
ignoredia bool // ignoredia value from last sort
locale string // locale value from last sort
noPerm bool // whether lf has no permission to open the directory
- lines []string // lines of text to display if directory previews are enabled
}
func newDir(path string) *dir {
@@ -187,7 +186,6 @@ func newDir(path string) *dir {
}
return &dir{
- loading: gOpts.dirpreviews, // Directory is loaded after previewer function exits.
loadTime: time,
path: path,
files: files,
@@ -450,7 +448,6 @@ type nav struct {
deleteCountChan chan int
deleteTotalChan chan int
previewChan chan string
- dirPreviewChan chan *dir
dirChan chan *dir
regChan chan *reg
fileChan chan *file
@@ -480,8 +477,6 @@ type nav struct {
}
func (nav *nav) loadDirInternal(path string) *dir {
- nav.startPreview()
-
d := &dir{
loading: true,
loadTime: time.Now(),
@@ -500,9 +495,6 @@ func (nav *nav) loadDirInternal(path string) *dir {
d := newDir(path)
d.sort()
d.ind, d.pos = 0, 0
- if gOpts.dirpreviews {
- nav.dirPreviewChan <- d
- }
nav.dirChan <- d
}()
return d
@@ -541,14 +533,10 @@ func (nav *nav) checkDir(dir *dir) {
return
}
- nav.startPreview()
dir.loading = true
dir.loadTime = now
go func() {
nd := newDir(dir.path)
- if gOpts.dirpreviews {
- nav.dirPreviewChan <- nd
- }
nav.dirChan <- nd
}()
case dir.sortby != getSortBy(dir.path) ||
@@ -599,7 +587,6 @@ func newNav(height int) *nav {
deleteCountChan: make(chan int, 1024),
deleteTotalChan: make(chan int, 1024),
previewChan: make(chan string, 1024),
- dirPreviewChan: make(chan *dir, 1024),
dirChan: make(chan *dir),
regChan: make(chan *reg),
fileChan: make(chan *file),
@@ -718,23 +705,6 @@ func (nav *nav) exportFiles() {
}
}
-func (nav *nav) dirPreviewLoop(ui *ui) {
- var prevPath string
- for dir := range nav.dirPreviewChan {
- if dir == nil && len(gOpts.previewer) != 0 && len(gOpts.cleaner) != 0 && nav.volatilePreview {
- cmd := exec.Command(gOpts.cleaner, prevPath)
- if err := cmd.Run(); err != nil {
- log.Printf("cleaning preview: %s", err)
- }
- nav.volatilePreview = false
- } else if dir != nil {
- win := ui.wins[len(ui.wins)-1]
- nav.previewDir(dir, win)
- prevPath = dir.path
- }
- }
-}
-
func (nav *nav) previewLoop(ui *ui) {
var prev string
for path := range nav.previewChan {
@@ -783,64 +753,6 @@ func matchPattern(pattern, name, path string) bool {
return matched
}
-func (nav *nav) previewDir(dir *dir, win *win) {
- defer func() {
- dir.loading = false
- nav.dirChan <- dir
- }()
-
- var reader io.Reader
-
- if len(gOpts.previewer) != 0 {
- cmd := exec.Command(gOpts.previewer, dir.path,
- strconv.Itoa(win.w),
- strconv.Itoa(win.h),
- strconv.Itoa(win.x),
- strconv.Itoa(win.y))
-
- out, err := cmd.StdoutPipe()
- if err != nil {
- log.Printf("previewing dir: %s", err)
- return
- }
-
- if err := cmd.Start(); err != nil {
- log.Printf("previewing dir: %s", err)
- out.Close()
- return
- }
-
- defer func() {
- if err := cmd.Wait(); err != nil {
- if e, ok := err.(*exec.ExitError); ok {
- if e.ExitCode() != 0 {
- nav.volatilePreview = true
- }
- } else {
- log.Printf("loading dir: %s", err)
- }
- }
- }()
- defer out.Close()
- reader = out
- buf := bufio.NewScanner(reader)
-
- for i := 0; i < win.h && buf.Scan(); i++ {
- for _, r := range buf.Text() {
- if r == 0 {
- dir.lines = []string{"\033[7mbinary\033[0m"}
- return
- }
- }
- dir.lines = append(dir.lines, buf.Text())
- }
-
- if buf.Err() != nil {
- log.Printf("loading dir: %s", buf.Err())
- }
- }
-}
-
func (nav *nav) preview(path string, win *win) {
reg := ®{loadTime: time.Now(), path: path}
defer func() { nav.regChan <- reg }()
diff --git a/ui.go b/ui.go
index eeae0cf..365d1f9 100644
--- a/ui.go
+++ b/ui.go
@@ -356,7 +356,7 @@ type dirStyle struct {
role dirRole
}
-func (win *win) printDir(ui *ui, dir *dir, context *dirContext, dirStyle *dirStyle, previewLoading bool) {
+func (win *win) printDir(ui *ui, dir *dir, context *dirContext, dirStyle *dirStyle) {
if win.w < 5 || dir == nil {
return
}
@@ -368,25 +368,11 @@ func (win *win) printDir(ui *ui, dir *dir, context *dirContext, dirStyle *dirSty
return
}
fileslen := len(dir.files)
- if (dir.loading && fileslen == 0) || (dirStyle.role == Preview && dir.loading && gOpts.dirpreviews) {
- if dirStyle.role != Preview || previewLoading {
- win.print(ui.screen, 2, 0, messageStyle, "loading...")
- }
+ if dir.loading && fileslen == 0 {
+ win.print(ui.screen, 2, 0, messageStyle, "loading...")
return
}
- if dirStyle.role == Preview && gOpts.dirpreviews && len(gOpts.previewer) > 0 {
- // Print previewer result instead of default directory print operation.
- st := tcell.StyleDefault
- for i, l := range dir.lines {
- if i > win.h-1 {
- break
- }
-
- st = win.print(ui.screen, 2, i, st, l)
- }
- return
- }
if fileslen == 0 {
win.print(ui.screen, 2, 0, messageStyle, "empty")
return
@@ -767,10 +753,10 @@ func (ui *ui) loadFile(app *app, volatile bool) {
return
}
- if curr.IsDir() {
- ui.dirPrev = app.nav.loadDir(curr.path)
- } else if curr.Mode().IsRegular() {
+ if curr.Mode().IsRegular() || (curr.IsDir() && gOpts.dirpreviews) {
ui.regPrev = app.nav.loadReg(curr.path, volatile)
+ } else if curr.IsDir() {
+ ui.dirPrev = app.nav.loadDir(curr.path)
}
}
@@ -1052,8 +1038,7 @@ func (ui *ui) draw(nav *nav) {
}
if dir := ui.dirOfWin(nav, i); dir != nil {
ui.wins[i].printDir(ui, dir, &context,
- &dirStyle{colors: ui.styles, icons: ui.icons, role: role},
- nav.previewLoading)
+ &dirStyle{colors: ui.styles, icons: ui.icons, role: role})
}
}
@@ -1081,12 +1066,11 @@ func (ui *ui) draw(nav *nav) {
if err == nil {
preview := ui.wins[len(ui.wins)-1]
- if curr.IsDir() {
- preview.printDir(ui, ui.dirPrev, &context,
- &dirStyle{colors: ui.styles, icons: ui.icons, role: Preview},
- nav.previewLoading)
- } else if curr.Mode().IsRegular() {
+ if curr.Mode().IsRegular() || (curr.IsDir() && gOpts.dirpreviews) {
preview.printReg(ui.screen, ui.regPrev, nav.previewLoading, &ui.sxScreen)
+ } else if curr.IsDir() {
+ preview.printDir(ui, ui.dirPrev, &context,
+ &dirStyle{colors: ui.styles, icons: ui.icons, role: Preview})
}
}
}
And it works with the following #!/bin/sh
if [ -d "$1" ]; then
tree "$1"
else
nl -ba "$1"
fiAnyway all of this discussion on So I'm still not sure whether Pros:
Cons:
Personally, I still lean towards returning an outdated list just because it's cleaner, and a user can always just run the |
|
Okay, that's a number of strange design decisions for that one option in particular, so I agree let's not deal with it here. And interesting find with Your patch seems to work okay. The only problem I see is some symbols not clearing up if emojis are in file names. But that's a problem with regular file preview too, so not really different. e.g. it's not too bad anyway, here one Ah, never mind, original Tested via if [ -d "$1" ]; then
echo "It's a dir!😃"
exa --color=always -1 "$1"
fi
#... do the file previewWhat if we just return e.g. That way, user can distinguish empty dir with loading in scripts too. Because otherwise e.g. |
|
I wouldn't worry about emojis, it's probably an issue with their width being reported incorrectly, possibly related to #1366. I'm tempted to commit that patch at some time in the future anyway (if I can't find any issues from testing), the implementation is much cleaner than the current one from #842.
If Regarding So after thinking about it some more, I guess it's fine to keep the current behavior you have for now. I think it's unlikely to matter either way in practice, and there isn't all that much extra complexity from a maintenance perspective. We can always revisit this later if a user asks about it. So yeah the changes look fine I will approve it. Thanks for your contribution! |
What it does
Implements #1084 as
lf -remote "query $id files".It outputs
\nseparated list of full file (dir/link/etc) paths of all files in lf's current directory visible at the moment with the current settings. (But it doesn't care about your scroll position)The benefit compared to using external sorting tools is that
filesmaintains the same sorting order and filter as used in lf at the moment.This means that you can continue to view your files in some external program, and the "next" and "previous" will mean the same thing as in lf, opening the same files as "j" and "k" in lf.
Example
Example usage with nsxiv (image viewer):
lfrc open nsxiv
The current state
So far, it works as expected, that is:
gState.data["files"]) the correct file pathson-...commands, typically just after launchBasically, it should look like what you see in lf, if you scrolled all the way and copied everything, and resolved full path for everything. If it doesn't, it's a bug.
TODO - this needs fixing or at least being addressed
set dirpreviews truethe current directory isn't doneloadinguntil the previewer script exits, even though it runs on directory's child, thus shouldn't really block it. And it walks all dirs from / to the viewed, increasing the delay. The patch from Add an ability to query files in the current directory as displayed in lf #1949 (comment) seems to fix it, switching to the existing regular file previewer function.Implementation
Old
It works by calling
app.nav.exportFilesInCurrDir()in a few key places, which updatesgState.data["files"]ifnavinitialized and the current directory is done loading.For convenience,
exportFilesInCurrDirtakesoriginof call as the argument that you can pass to logs, but I don't expect we keep it (too noisy).To choose the places of calls, I've gone over references of
dir.sort(), avoiding exporting right after, and instead trying to find some higher level function where possible to avoid needless exports.It seems like I could almost solely rely on export where
dirChanis consumed. I explicitly added some exports just in case there are some corner cases I didn't account for, but it seems likerename,onChdir,nav.sortare covered byloop.case app.nav.dirChanexport.The only thing it doesn't cover is
setFilter, which I added too.Original implementation mentioned the above that always maintains
filesin sync with what is displayed in lf can be found here:https://github.com/MahouShoujoMivutilde/lf/tree/query-files-always-in-sync
(force-pushed out of this branch to not clutter history)
Replaced with the proposed export in
runShell(), as it works fine, and when it has problems (set dirpreviews true) - mine does too.So, this is where we're at. It works fine for me. At this point it will be more productive to work on it in pr, should more changes be needed.