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

First selection shouldn't be to navigate back #86

Closed
kkharji opened this issue Jan 26, 2022 · 9 comments
Closed

First selection shouldn't be to navigate back #86

kkharji opened this issue Jan 26, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@kkharji
Copy link
Member

kkharji commented Jan 26, 2022

Is your feature request related to a problem? Please describe.

I'm always frustrated when I open the browser or navigate to a directory and have the selection being to jump back

Describe the solution you'd like

First selection is on the first result not on the action to go to parent.

Describe alternatives you've considered

I played around with the default action tried telescope actions like selection_move and move_next but it didn't work.

@kkharji kkharji added the enhancement New feature or request label Jan 26, 2022
@fdschmidt93
Copy link
Member

fdschmidt93 commented Jan 27, 2022

Isn't that essentially achieved with :Telescope file_browser default_selection_index=2? And of course you can pass default_selection_index = 2 to opts of lua function call and extension setup analogously (also just tried it).

I'm confident enough that I can close this 😅 Let me know otherwise :)

This is also much preferred because users then can have it as they want. I have not clue whether that should be the default behavior, I most often fuzzy search anyways.

Maybe this could be added to the wiki or something. Once I find some more motivation/time to work on file browser, I'll tidy up the wiki.

@kkharji
Copy link
Member Author

kkharji commented Jan 27, 2022

NICE! indeed it fix my issue. it's kind annoying in that you could see the highlighter jump down after switching but it's good enough. The reason I'm using it like that is because usually when browsing files, I don't do fuzzy match because I'm reviewing files one by one, like I would do with ranger.

I have not clue whether that should be the default behavior, I most often fuzzy search anyways.

him yah hmm I'd vote for it to be default behavior with finding a work around for the noticeable switch to index 2.

Thanks @fdschmidt93

@fdschmidt93
Copy link
Member

fdschmidt93 commented Jan 27, 2022

It's kind annoying in that you could see the highlighter jump down after switching but it's good enough.

What do you mean? When I open the file browser, I don't see any flickering of sorts. Otherwise, it's a telescope upstream issue if you look at pickers.lua how selection is initially set on the first entry and then set anew on once finding is completed (very fast w/o prompt anyways), assuming I'm reading the code correctly at a quick glance. In any case, upstream issue. But even then, on initial sorting it should always be plenty fast (unless depth=false or something like that is set).

Nothing I could do from within telescope-file-browser.nvim would be able to change that as I'd only be able to infer the selection index via some loop (which is not needed..). In other words, I'd practically always be using the same logic of setting a selection.

him yah hmm I'd vote for it to be default behavior with finding a work around for the noticeable switch to index 2.

Fine by me I guess, could you please make a PR? :) Would have to be added to pconf of https://github.com/nvim-telescope/telescope-file-browser.nvim/blob/master/lua/telescope/_extensions/file_browser.lua

@kkharji
Copy link
Member Author

kkharji commented Jan 27, 2022

What do you mean? When I open the file browser, I don't see any flickering of sorts.

it's barely noticeable but there is an initial indication of the movement the selection.

asciicast

Otherwise, it's a telescope upstream issue if you look at pickers.lua how selection is initially set on the first entry and then set anew on once finding is completed (very fast w/o prompt anyways), assuming I'm reading the code correctly at a quick glance. In any case, upstream issue. But even then, on initial sorting it should always be plenty fast (unless depth=false or something like that is set).

Yes, definitely an upstream thing. also cursor the stop filkers. hmm I should open an issue for this to track it

Nothing I could do from within telescope-file-browser.nvim would be able to change that as I'd only be able to infer the selection index via some loop (which is not needed..). In other words, I'd practically always be using the same logic of setting a selection.

Or maybe an option to hide ../ all together, since in many cases it isn't needed with the intuitive mappings in #65. Any clue where I can control the the visibility of ../?

@fdschmidt93
Copy link
Member

fdschmidt93 commented Jan 27, 2022

Any clue where I can control the the visibility of ../

Not yet an option. PR welcome :) (E: though Idk, with default_selection_index = 2 it should be solved, rather than introducing yet another option.. mhm, not sure why it's so sluggish here). Sorry, I'm just quite lazy atm as I'm personally also largely happy with telescope-file-browser and worked plenty on it the past few weeks 😅 And then I'm happy if anyone capable chimes in :)

it's barely noticeable but there is an initial indication of the movement the selection.

I'd say it's quite noticeable actually; strange. For me that kind of flickering doesn't even occur in the slightest -- it's always instant. Even if I turn CPU freq to super low. I also tried the 0.6.1 appimage and doesn't occur for me there. Not sure why it differs so dramatically for me.

@kkharji
Copy link
Member Author

kkharji commented Jan 27, 2022

Sorry, I'm just quite lazy atm

hhhh no issues, I was planning to open a PR, but I was asking if there's a specific place where I can inject the new option. no worries.

worked plenty on it the past few weeks

Yah I appreciate that a lot, there was multiple efforts to get this project on it feats 😆 but now it's in a great state.

I'd say it's quite noticeable actually; strange. For me that kind of flickering doesn't even occur in the slightest -- it's always instant. Even if I turn CPU freq to super low. I also tried the 0.6.1 appimage and doesn't occur for me there. Not sure why it differs so dramatically for me.

I'd say it's macos and m1 faults 🤣 should've listened to conni and not bought this machine.

@fdschmidt93
Copy link
Member

hhhh no issues, I was planning to open a PR, but I was asking if there's a specific place where I can inject the new option. no worries.

Would be here:

fb_finders.browse_files = function(opts)
opts = opts or {}
-- returns copy with properly set cwd for entry maker
local entry_maker = opts.entry_maker { cwd = opts.path, path_display = { "tail" } }
if has_fd and opts.grouped == false then
local args = { "-a" }
if opts.hidden then
table.insert(args, "-H")
end
if opts.respect_gitignore == false then
table.insert(args, "--no-ignore-vcs")
end
if opts.add_dirs == false then
table.insert(args, "--type")
table.insert(args, "file")
end
if type(opts.depth) == "number" then
table.insert(args, "--maxdepth")
table.insert(args, opts.depth)
end
return async_oneshot_finder {
fn_command = function()
return { command = "fd", args = args }
end,
entry_maker = entry_maker,
results = { entry_maker(Path:new(opts.path):parent():absolute()) },
cwd = opts.path,
}
else
local data = scan.scan_dir(opts.path, {
add_dirs = opts.add_dirs,
depth = opts.depth,
hidden = opts.hidden,
})
if opts.path ~= os_sep then
table.insert(data, 1, Path:new(opts.path):parent():absolute())
end
if opts.grouped then
fb_utils.group_by_type(data)
end
return finders.new_table { results = data, entry_maker = entry_maker }
end
end

But I'm torn, wouldn't be default_selection_index = 2 pretty much more than fine for 95%+ of users rather than introducing another option? Mhm.

@kkharji
Copy link
Member Author

kkharji commented Jan 27, 2022

Hmmm yah, idk, on one note, it is aesthetically pleasing to remove things you don't use, on the other hand it introduce yet another option. Given that this the default view of most terminal file browser, I believe this is an important option to have.

@fdschmidt93
Copy link
Member

Ok :)

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

No branches or pull requests

2 participants