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

Add mouse support for selecting and opening files #963

Merged
merged 3 commits into from
Oct 19, 2022

Conversation

p-ouellette
Copy link
Contributor

@p-ouellette p-ouellette commented Oct 14, 2022

Left click to select, right click to open, similar to ranger. Closes #919.
Mouse mappings work as before and override the default behavior.
Mouse is disabled in command-line mode.

for i := 0; i < wins; i++ {
if dir := ui.dirOfWin(nav, i); dir != nil {
ui.wins[i].printDir(ui.screen, dir, nav.selections, nav.saves, nav.tags, ui.styles, ui.icons)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No functional change here, just refactored to use dirOfWin.

}

var file *file
ind := dir.ind - dir.pos + y - w.y
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw max(dir.ind-dir.pos, 0) elsewhere, but I'm pretty sure dir.pos will never be greater than dir.ind.

@p-ouellette p-ouellette marked this pull request as ready for review October 15, 2022 07:05
@gokcehan
Copy link
Owner

@p-ouellette Can you update this patch to resolve conflicts?

@ilyagr
Copy link
Collaborator

ilyagr commented Oct 16, 2022

@p-ouellette I tried this out, and it's really nice, but there seem to either be some bug or this triggers some tcell/tmux bug. I get problems if I either click or scroll the mouse wheel while the mouse is over the lf preview window (previewing a file). After that, no matter where the mouse is, scrolling the wheel seems to open either the file or the previewer (in my setup, open and preview commands are very similar).

This is in tmux with set mouse on in both tmux and lf on kitty. After a bit of this madness, tmux seemed to think a button is pressed even when it isn't, even in other panes (I'm not sure how to reproduce this part).

If you can't reproduce this, I can look in more details to either send you my config or find the problem.

@ilyagr
Copy link
Collaborator

ilyagr commented Oct 17, 2022

I think I narrowed down the bug I described above and figured out why you didn't catch it, @p-ouellette. It has to do with the interaction of your code and the mouse support in less.

The config necessary to reproduce it is (in lf) set mouse; cmd open ${{less "$f"}}.

To reproduce the bug, start lf as LESS=--mouse lf (or add --mouse argument to less in the config above).
If you do not enable the mouse for less, e.g. by using LESS= lf, the bug does not happen.

I still don't know the exact problem, but it feels like maybe lf sees the "mouse down" event and not the "mouse up" event.

ui.go Outdated
@@ -932,8 +951,8 @@ func (ui *ui) draw(nav *nav) {
preview := ui.wins[len(ui.wins)-1]

if curr.IsDir() {
preview.printDir(ui.screen, ui.dirPrev, &context,
&dirStyle{colors: ui.styles, icons: ui.icons, previewing: true})
style.previewing = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor nit: I'm not a fan of this refactoring. I prefer as many variables as possible to be immutable, so that the reader doesn't have to pay careful attention to how it might change on them. This makes it seem that there's some important and confusing logic that will use the value of style below.

Copy link
Collaborator

@ilyagr ilyagr Oct 17, 2022

Choose a reason for hiding this comment

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

If you don't like the original version, I would prefer:

previewStyle := dirStyle{colors: style.colors, icons: style.icons, previewing: true}
preview.printDir(..., previewStyle)

I'm unsure if Go provides a shorter way to write that.

@p-ouellette
Copy link
Contributor Author

Rebased and updated for dirpreviews option.

@ilyagr thanks for testing. I can reproduce the bug. tcell is sending a button click event instead of a scroll event. The problem seems to be here: https://github.com/gdamore/tcell/blob/b9dc8a651aa1e7e564025ceb6bfa9a0f8fbceee4/tscreen.go#L1225. After clicking to open less, the button release event is read by less, so lf doesn't get it and wasbtn doesn't get set to false. I'll investigate fixing the bug in tcell.

@ilyagr
Copy link
Collaborator

ilyagr commented Oct 17, 2022

@p-ouellette Naively, maybe it would help to make lf do things on mouse button release rather than on button press?

Update: Another potential, naive, workaround: after lf runs an external program and it finishes, do something that resets tcell's mouse logic, e.g. turn the mouse events off and then on again.

Commit d3cbfcf fixes a bug where mouse wheel events were misdelivered as
button events.
@p-ouellette
Copy link
Contributor Author

The tcell bug is fixed: gdamore/tcell@d3cbfcf. I updated the dependency in lf to that commit.

@gokcehan
Copy link
Owner

@p-ouellette Thanks for the patch.

@gokcehan gokcehan merged commit a94015d into gokcehan:master Oct 19, 2022
@ilyagr
Copy link
Collaborator

ilyagr commented Oct 19, 2022

Nice! It works great for me so far.

We should probably update to an official release of tcell version once it appears.

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.

Select item on mouse click
3 participants