lf icon indicating copy to clipboard operation
lf copied to clipboard

`select` command always selects top file when `dircache` option is disabled

Open itsdeadguy opened this issue 2 years ago • 4 comments

I've been having some issues with select for some time now. Namely, it wouldn't work as intended. I finally did some sleuthing and discovered that my nodircache setting was impacting it.

Is this the way it's supposed to work? i initally set it up that way because i was worried the cache would become large/unwieldy and introduce delays

itsdeadguy avatar Sep 05 '23 14:09 itsdeadguy

Thanks for reporting. I did some investigation and it turns out this is due to the implementation of how lf loads/reloads directories.

Every directory is represented by a data structure in lf, which contains information like the list of files inside, the currently selected file, among other things. When a directory is reloaded, the old data structure is replaced by a new one. The old data structure is maintained in the directory cache, so if dircache is enabled, it can be used to maintain the currently selected file. However if dircache is disabled, this information is not saved, and the new directory will not know which file to select, and defaults to leaving the cursor at the top.

The same mechanism is used when running the select command. lf will change to the directory containing the selected file, which requires creating a new data structure to represent that directory. However directories can take a small but significant time to load, so the actual loading of the directory is done asynchronously in the background to avoid locking the UI. Therefore lf saves the name of the selected file in the directory cache, and when the directory is actually loaded, it will read from there to select the correct file.

I guess it would be possible to fix this by saving the name of the selected file outside the cache, but it would add some complexity to the code and I'm not sure if it is worth doing. The dircache option is enabled by default, is there any real reason why you disabled it? I'm not sure of your usage requirements, but I don't think the directory cache should take up much memory.


BTW it looks like the dircache option was introduced in #673, and the reason was not because of performance concerns, but actually because the cache was deemed unreliable (see the bug report in #627). However I actually fixed this bug myself in #1138, so I'm not even sure if the dircache option is needed anymore.

Pinging @gokcehan to see if there's anything else to add to the discussion.

joelim-work avatar Sep 06 '23 04:09 joelim-work

@joelim-work Thanks for investigating the issue. I don't have much to add this discussion. It is unfortunate that we ended up introducing the dircache option instead of fixing the renaming issue properly. I don't know if there is another usecase for dircache. If we decide to remove or deprecate this option, then it could be a good idea to ping the people in the original issue first to see if they are ok with this. Since they never said anything so far, they are most likely not around anymore and/or they don't use the select command themselves. I think it is probably easier to keep the dircache option at this point and instead maybe we can add a note about unexpected issues with the use of this option to the documentation.

@itsdeadguy I don't think it is a good idea to consider setting nodircache to increase the performance, since the whole point of caching is to increase the performance. The trade-off here is the additional memory usage, but I don't expect it to become an issue either. I wouldn't be surprised if the web browser you use to reply to this thread uses 100 times more memory than lf. If you somehow encounter an excessive memory usage at some point, remember you can always use reload (default <c-r>) to flush the cache manually, though the actual memory reclaim is only performed after the garbage collector.

gokcehan avatar Sep 07 '23 16:09 gokcehan

I guess it's worth pinging @sigasigasiga about this issue, I notice on their dotfiles that dircache is actually enabled, but that might not mean anything.

Anyway I think we can keep this issue open, I will tidy it up a bit.

joelim-work avatar Sep 08 '23 09:09 joelim-work

@itsdeadguy I don't think it is a good idea to consider setting nodircache to increase the performance, since the whole point of caching is to increase the performance. The trade-off here is the additional memory usage, but I don't expect it to become an issue either. I wouldn't be surprised if the web browser you use to reply to this thread uses 100 times more memory than lf. If you somehow encounter an excessive memory usage at some point, remember you can always use reload (default <c-r>) to flush the cache manually, though the actual memory reclaim is only performed after the garbage collector.

Thank you for clarifying this. I'm satisfied with the answers so far, but i'll keep an eye on the issue to see how it goes.

itsdeadguy avatar Sep 11 '23 00:09 itsdeadguy