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

Inactive tabs lose their sorting #1852

Open
2 of 3 tasks
dawsers opened this issue Oct 29, 2024 · 7 comments · Fixed by #1885
Open
2 of 3 tasks

Inactive tabs lose their sorting #1852

dawsers opened this issue Oct 29, 2024 · 7 comments · Fixed by #1885
Labels
feature New feature request

Comments

@dawsers
Copy link

dawsers commented Oct 29, 2024

yazi --debug output

Yazi
    Version: 0.3.3 (5eabd7d 2024-10-28)
    Debug  : true
    OS     : linux-x86_64 (unix)

Ya
    Version: 0.3.3 (5eabd7d 2024-10-28)

Emulator
    Emulator.via_env: ("xterm-kitty", "")
    Emulator.via_csi: Ok(Kitty)
    Emulator.detect : Kitty

Adapter
    Adapter.matches: Kgp

Desktop
    XDG_SESSION_TYPE           : Some("wayland")
    WAYLAND_DISPLAY            : Some("wayland-1")
    DISPLAY                    : Some(":0")
    SWAYSOCK                   : None
    HYPRLAND_INSTANCE_SIGNATURE: Some("4520b30d498daca8079365bdb909a8dea38e8d55_1730133172_1403669487")
    WAYFIRE_SOCKET             : None

SSH
    shared.in_ssh_connection: false

WSL
    WSL: false

Variables
    SHELL              : Some("/usr/bin/bash")
    EDITOR             : Some("/usr/bin/nvim")
    VISUAL             : Some("/usr/bin/nvim")
    YAZI_FILE_ONE      : None
    YAZI_CONFIG_HOME   : None

Text Opener
    default     : Some(Opener { run: "${EDITOR:-vi} \"$@\"", block: true, orphan: false, desc: "$EDITOR (block)", for_: None, spread: true })
    block-create: Some(Opener { run: "${EDITOR:-vi} \"$@\"", block: true, orphan: false, desc: "$EDITOR (block)", for_: None, spread: true })
    block-rename: Some(Opener { run: "${EDITOR:-vi} \"$@\"", block: true, orphan: false, desc: "$EDITOR (block)", for_: None, spread: true })

Multiplexers
    TMUX               : false
    tmux version       : No such file or directory (os error 2)
    tmux build flags   : enable-sixel=Unknown
    ZELLIJ_SESSION_NAME: None
    Zellij version     : No such file or directory (os error 2)

Dependencies
    file             : 5.45
    ueberzugpp       : No such file or directory (os error 2)
    ffmpegthumbnailer: 2.2.3
    magick           : 7.1.1-39
    fzf              : 0.55.0
    fd               : 10.2.0
    rg               : 14.1.1
    chafa            : 1.14.2
    zoxide           : 0.9.6
    7z               : 17.05
    7zz              : No such file or directory (os error 2)
    jq               : 1.7.1

Please describe the problem you're trying to solve

This is not a relevant issue in yazi given a regular use. Active tabs are always correctly sorted.

I have a plugin that creates a dual pane configuration, dual-pane.yazi, and the tab shown on the non-active pane loses its sorting in certain cases, like for example when adding a new tab or copying files into it.

Would you be willing to contribute this feature?

  • Yes, I'll give it a shot

Describe the solution you'd like

Going through Yazi's code, yazi-core/src/manager/commands/update_files.rs:44
applies the file attributes to the active tab:

self.active_mut().apply_files_attrs();

If tI change his code to apply the file attributes to all the tabs, the "problem" disappears:

for tab in self.tabs.iter_mut() {
    tab.apply_files_attrs();
}

I don't claim this is the best solution, or even that a solution is needed, because this only affects a handful of users than want to show more than one tab at the same time, but it would be great if you could help finding a solution that is maybe better than what I am proposing, as I don't know the code very well, and I am sure there are better ways to do this without a noticeable performance hit.

Thank you!

Additional context

No response

Validations

  • I have searched the existing issues/discussions
  • The latest nightly build doesn't already have this feature
@dawsers dawsers added the feature New feature request label Oct 29, 2024
@sxyazi
Copy link
Owner

sxyazi commented Oct 29, 2024

Could you please provide a minimal reproducible code for me to tinker with and see if I can make it work? If showing two tabs at the same time is tricky, just printing out the file lists for the different tabs would be enough.

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Oct 29, 2024
@dawsers
Copy link
Author

dawsers commented Oct 29, 2024

Of course.

This is a minimal plugin that creates two visible tabs and shows the problem:

sorting-bug.yazi

local view = false
local old_root_layout = nil
local old_root_build = nil
local left
local right

local function entry(state, args)
  local action = args[1]
  if not action then
    return
  end

  if action == "toggle" then
    if not view then
      -- Create tabs
      left = cx.tabs.idx
      right = left
      old_root_layout = Root.layout
      old_root_build = Root.build
      Root.layout = function(root)
        root._chunks = ui.Layout()
          :direction(ui.Layout.HORIZONTAL)
          :constraints({
            ui.Constraint.Percentage(50),
            ui.Constraint.Percentage(50),
          })
          :split(root._area)
      end
      Root.build = function(root)
        root._children = {
          Tab:new(root._chunks[1], cx.tabs[left]),
          Tab:new(root._chunks[2], cx.tabs[right]),
        }
      end
      view = true
    else
      view = false
      left = cx.tabs.idx
      right = left
      -- Reset UI
      Root.layout = old_root_layout
      Root.build = old_root_build
    end
    ya.app_emit("resize", {})
  end

  if action == "bug" then
    ya.manager_emit("tab_create", {})
    right = right + 1
  end
end

local function setup(state, opts)
end

return {
  entry = entry,
  setup = setup,
}
  1. Install the plugin
  2. Assign a key to each of the two actions, for example:
[manager]
prepend_keymap = [
    { on = "T", run = "plugin --sync sorting-bug --args=toggle", desc = "Call sorting-bug" },
    { on = "B", run = "plugin --sync sorting-bug --args=bug", desc = "Call sorting-bug" },
]
  1. Change yazi's configuration to use a sorting mode
sort_by = "mtime"
sort_reverse = true
sort_dir_first = false
show_hidden = true
  1. Start yazi
  2. Toggle the dual tab view by hitting T (toggle). Now both tabs will show the same directory, and their files will be properly sorted.
  3. Hit B to call the bug action. It will create a new tab and show it in the right pane. The files on that tab will be sorted correctly, as it is the active tab. The other pane, instead, will be showing the old active tab, and it will lose its sorting. You can see the parent also loses its sorting.

Applying the patch in my original message "fixes" the problem by forcing all the tabs to get their files updated, but I doubt it is the most efficient method.

Aside from this, I think there is also a bug when using mtime sorting because of folder contents caching. When I copy a file to a directory, the mtime of the directory changes, but yazi doesn't notice and still shows the original time (before the copy). If I exit yazi and start it again (or create a new tab in the same directory), it reloads the directory and the time is correct.

@github-actions github-actions bot removed the waiting on op Waiting for more information from the original poster label Oct 29, 2024
@sxyazi
Copy link
Owner

sxyazi commented Oct 31, 2024

Thanks, I replicated it and made a bit of simplification to the reproducer:

local function entry(_, args)
  if args[1] == "bug" then
    ya.manager_emit("tab_create", {})
    return
  end

  Root.layout = function(root)
    root._chunks = ui.Layout()
      :direction(ui.Layout.HORIZONTAL)
      :constraints({
        ui.Constraint.Percentage(50),
        ui.Constraint.Percentage(50),
      })
      :split(root._area)
  end
  Root.build = function(root)
    root._children = {
      Tab:new(root._chunks[1], cx.tabs[1]),
      Tab:new(root._chunks[2], cx.tabs[#cx.tabs]),
    }
  end
  local redraw = Tab.redraw
  Tab.redraw = function(self)
    self._base = { ui.Text(string.format("ID: %d; Active: %d", self._tab.id, cx.tabs.idx)):area(self._area) }
    return redraw(self)
  end
  ya.app_emit("resize", {})
end

return { entry = entry }

This is a result of the combination of two optimizations:

  • Yazi tries to reuse I/O results as much as possible. When you create a new tab with the same path as the current directory, it does a complete I/O directory read and applies the results to all tabs with that path, since applying those results is much cheaper than doing an I/O read.
  • Yazi minimizes computation. The data returned by the filesystem is unordered, so it needs to be sorted. To reduce the amount of computation, it only sorts the currently active tab, which means that in dual-pane mode, the other tab doesn't get sorted at all.

Yazi always treats the cx.tabs.idx as the active tab but doesn't know about the other tab in dual-pane mode, so in this case, you need to manually trigger sorting from the plugin - I can add a new tab parameter to the sort command to specify the ID of another tab to trigger sorting for.

@dawsers
Copy link
Author

dawsers commented Oct 31, 2024

Thank you. It is more or less what I was able to figure out by looking at the code.

If I follow your advice and do a tab_switch to the inactive pane, sort, and then tab_switch to the active pane, it still doesn't work. I think adding a tab parameter to sort would be the best compromise in terms of performance, as it can be triggered only on demand, but I don't know if that will make this work, as the manual way is not working.

It seems despite the sorting happening, it then gets discarded as I switch back to the other pane.

Thanks a lot.

EDITED: I thought switching, sorting and then switching back was working, but it is not.

@sxyazi
Copy link
Owner

sxyazi commented Nov 3, 2024

Please try #1885

With this PR, you can specify which tab to apply sorting to by:

ya.manager_emit('sort', { tab = 1 })

(replace 1 as an actual tab ID.)

@dawsers
Copy link
Author

dawsers commented Nov 3, 2024

Thank you for adding this feature.

Unfortunately, as far as I can tell, this doesn't fix the case I was presenting. Maybe the reason is caching seems to have some issues.

  1. When I create the tab, both panes point to the same directory, but to different tabs. The active pane gets sorted, as seen in the Rust code I mentioned (update_files.rs). The other pane is not sorted even if I trigger sorting manually using the new option to sort.
  2. mtime seems to be broken in the caching system. If I copy a file to a directory, its mtime should be updated to the time of the action. The caching system doesn't trigger a reload and keeps the original mtime for the directory. I can move out of the directory, back in, etc, and it won't trigger a reload (probably because it still thinks the cache is valid). But if I exit yazi or delete all the tabs referencing the directory and go back in, its mtime is correct again.

@sxyazi
Copy link
Owner

sxyazi commented Nov 4, 2024

Will look into it. Reopened

@sxyazi sxyazi reopened this Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants