Feature: Filter on process list #136
Conversation
src/tui/proc-filter.go
Outdated
| pv.searchProcess(searchTerm, isRegex, caseSensitive) | ||
| } | ||
| f.GetFormItem(0).(*tview.InputField).SetChangedFunc(func(_ string) { | ||
| searchFunc() |
There was a problem hiding this comment.
Can you pass the text to the searchFunc here?
searchFunc := func(searchTerm string) {
caseSensitive := f.GetFormItem(1).(*tview.Checkbox).IsChecked()
isRegex := f.GetFormItem(2).(*tview.Checkbox).IsChecked()
pv.searchProcess(searchTerm, isRegex, caseSensitive)
}
f.GetFormItem(0).(*tview.InputField).SetChangedFunc(func(text string) {
searchFunc(text)
})
src/tui/proc-filter.go
Outdated
| f.SetButtonsAlign(tview.AlignCenter) | ||
| f.SetTitle("Search Process") | ||
|
|
||
| f.AddInputField("Search For", pv.logsText.getSearchTerm(), fieldWidth, nil, nil) |
There was a problem hiding this comment.
pv.logsText.getSearchTerm()Looks like a copy-paste from the log search 😉
It should be the current search term if wasn't reset.
src/tui/proc-table.go
Outdated
| @@ -46,6 +47,12 @@ func (pv *pcView) fillTableData() { | |||
| pv.procTable.RemoveRow(row) | |||
| continue | |||
| } | |||
|
|
|||
| if pv.processRegex != nil && !pv.processRegex.MatchString(state.Name) { | |||
There was a problem hiding this comment.
I think pv.processRegex should be protected with a mutex.
It's being read and written from 2 different threads.
|
I am one of said users with many processes, which is why i wanted this feature 😄 I've fixed the remarks. And added a mutex. But it doesn't seem to fix the issue. The issue also doesn't happen during the search (when the regex is being written). But happens only if you submit the search with an empty result set. Which makes me suspect it's got something to do with the log window updating and doing something with the selected process, when no process is selected |
stijnveenman
force-pushed
the
feature/proc-filter
branch
from
9f5f5c1 to
2f8e269
Compare
|
Looking into it again, i found the issue. It was actually caused if i change the row on an empty table. Causing the selection to change to a non existing row. I tested this on an empty tview project and found the same issue. rivo/tview#944 Added a workaround for now by setting the table to be non selectable if no selectable rows are present. |
src/tui/proc-table.go
Outdated
| } | ||
|
|
||
| func (pv *pcView) searchProcess(search string, isRegex, caseSensitive bool) error { | ||
| pv.processRegexMtx.Lock() |
There was a problem hiding this comment.
That's a good start that you added a mutex to the regex assignment, but the use of the pv.processRegex is still unprotected during match.
What you'll probably want to do is implement 2 funcs:
func (pv *pcView) setProcRegex(reg *regexp.Regexp) {
pv.processRegexMtx.Lock()
defer pv.processRegexMtx.Unlock()
//it can be nil when you want to reset
pv.processRegex = reg
}
func (pv *pcView) matchProcRegex(procName string) bool {
pv.processRegexMtx.Lock()
defer pv.processRegexMtx.Unlock()
if pv.processRegex != nil {
return pv.processRegex.MatchString(procName)
}
return false
}There was a problem hiding this comment.
That makes sense, i do wonder. Does this need a mutex?
The proc-filter form creates a new regex for every input, and saves the pointer to pv.processRegex.
Then the fillTableData gets the current pointer to the current regex. If another search input it done during the fillTableData call, a new regex is created and written to pv. fillTableData still points to the old regex though, and this regex is not garbage collected. So at worst its slightly outdated?
There was a problem hiding this comment.
2 different threads writing to the same memory location, even for a new value assignment, is a recipe for disaster :)
| pv.procTable.SetSelectable(false, false) | ||
| } else { | ||
| pv.procTable.SetSelectable(true, false) | ||
| } |
There was a problem hiding this comment.
I think you encountered my bug, which I already fixed in my env.
Instead of this block, can you try:
if pv.procTable.GetRowCount()-1 > row - 1{Notice the row - 1
In the line below.
There was a problem hiding this comment.
I added this block as a workaround for the issue in tview. It seems to break if the currently selected row is removed from the table. Which i was also able to replicate in isolation.
At one point i tried to fix it by calling table.clear() before rendering the rows, which means it didn't have to clean up the remaining rows. And it still had the same issue (with the lines you mentioned removed).
I tried it with this change and run into the same issue. (to test it)
- enter a filter that results in no processes
- press j or k
There was a problem hiding this comment.
That's strange. I couldn't replicate it in my environment by, for example, hiding a selected namespace Ctrl+G.
Even if it's the last row, leaving me with 0 rows (except the title row).
There was a problem hiding this comment.
Interesting. How did you filter all rows using namespaces? Doesn't every namespace in the list have at least one project?
There was a problem hiding this comment.
I am cheating :) I also have the implementation of #133
I have 2 processes:
- ns: aaa, disabled
- ns: default
I select theaaanamespace and toggle disabled procs. Leaving me with an empty list:
There was a problem hiding this comment.
Yes sorry, that's what i ment by "crashing" , actually the background processes also keep running fine. (As seen by the logs updating by looking at the logfile directly)
There was a problem hiding this comment.
I attached a goroutine profiler. It looks like the table is stuck in this loop:
https://github.com/rivo/tview/blob/861aa94d61c899b32a1304d61b840d0178ece408/table.go#L1373
# 0x65ae14 github.com/rivo/tview.(*Table).InputHandler.func1.2+0x54 /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/table.go:1375
# 0x65aca3 github.com/rivo/tview.(*Table).InputHandler.func1.5+0xa3 /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/table.go:1440
# 0x65a5a4 github.com/rivo/tview.(*Table).InputHandler.func1+0x4e4 /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/table.go:1598
# 0x65a08f github.com/rivo/tview.(*Table).InputHandler.(*Box).WrapInputHandler.func2+0x4f /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/box.go:167
# 0x63bb16 github.com/rivo/tview.(*Grid).InputHandler.func1+0x1b6 /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/grid.go:277
# 0x63b92f github.com/rivo/tview.(*Grid).InputHandler.(*Box).WrapInputHandler.func2+0x4f /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/box.go:167
# 0x650801 github.com/rivo/tview.(*Pages).InputHandler.func1+0xa1 /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/pages.go:311
# 0x65072f github.com/rivo/tview.(*Pages).InputHandler.(*Box).WrapInputHandler.func2+0x4f /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/box.go:167
# 0x627737 github.com/rivo/tview.(*Application).Run+0x557 /home/eugene/go/pkg/mod/github.com/rivo/tview@v0.0.0-20240101144852-b3bd1aa5e9f2/application.go:344
# 0xb3a3fd github.com/f1bonacc1/process-compose/src/tui.SetupTui+0x13d /home/eugene/projects/go/process-compose/src/tui/view.go:474
# 0xb7ebb8 github.com/f1bonacc1/process-compose/src/cmd.startTui+0x178 /home/eugene/projects/go/process-compose/src/cmd/project_runner.go:92
There was a problem hiding this comment.
Given this remaining issue, is there anything else in this PR that still needs to be addressed?
Do you think this workaround should be used for now? GIven that #133 will have a similar issue? Or wait for an upstream fix?
There was a problem hiding this comment.
I already merged your PR locally.
I believe I will release a new version over the weekend.
Does this timeline work for you?
There was a problem hiding this comment.
Awesome! no rush, i was just wondering if there were any more changes needed 👌
|
Add the ability to filter on process list using
/(vim keybinding).Can be confirmed with enter (has the same issue as #135)
Or cancelled with Escape. On start it resets the search, and searches as you type to see the result.
If cancelled early it will reset the search.
Screen.Recording.2024-02-04.at.21.00.54.mov
I did run into one issue where, if the search results in an empty list, it hangs the process. The processes are still running in the background (seen by the logs updating). But the TUI doesn't respond anymore, i haven't been able to figure out what the reasoning is behind this. If i click escape quickly (to cancel the search), the problem doesn't show up.