feat(#2364): add option to show files first

[Akmadan23]

This is a simple and straight forward implementation of the feature requested in #2364. For the configuration I think that since sort.folders_first is enabled by default, sort.files_first = true should override it. I made a minor refactoring in explorer/sorter.lua, moving the folders-first logic in a separate function so that it's easier to manage and enhance.

[alex-courtis]

FYI: you're a repo member: you can push a branch to this repo instead of using a fork.

[alex-courtis]

This is nice, simplifies things a little as well.

Tested:

• all permutations of folders_first and files_first
• sort function

[alex-courtis]

Could we flip this around? Test this config at the start of the function?

That would remove the need for the mystery x

[Akmadan23]

Yeah, I stored in x what should be returned if sort.folders_first is enabled and reverse it with sort.files_first, but now that I think about it maybe something like this would be more readable and intuitive.

local a_and_not_b, not_a_and_b

if M.config.sort.files_first then
  a_and_not_b = false
  not_a_and_b = true
elseif M.config.sort.folders_first then
  a_and_not_b = true
  not_a_and_b = false
else
  return
end

if a.nodes and not b.nodes then
  return a_and_not_b
elseif not a.nodes and b.nodes then
  return not_a_and_b
elseif a.nodes and b.nodes and third_option_callback ~= nil then
  return third_option_callback()
end

[alex-courtis]

Much more readable. Could we go simpler?

if a.nodes and not b.nodes then
  return M.config.sort.folders_first

[Akmadan23]

After several tests I think that would not work because the function should return true or false only if one (or both) of sort.folders_first and sort.files_first are enabled, otherwise it should return nil and continue without altering entries' order.

[alex-courtis]

Ah of course. How about an early exit?

if not M.config.sort.files_first and not M.config.sort.folders_first then
  return
end

[Akmadan23]

Ok, this is the simplest and most concise way I got it to work:

if not (M.config.sort.folders_first or M.config.sort.files_first) then
  return
end

local x = M.config.sort.files_first or not M.config.sort.folders_first

if not a.nodes and b.nodes then
  return x
elseif a.nodes and not b.nodes then
  return not x
elseif fallback and a.nodes and b.nodes then
  return fallback()
end

Now we just have to give a sensible name to x.

[alex-courtis]

This should do it:

---Evaluate `sort.folders_first` and `sort.files_first`, files takes precedence.
---@param a table node
---@param b table node
---@param fallback function|nil function should return `boolean`. Evaluated when both are folders.
---@return boolean|nil
local function folders_or_files_first(a, b, fallback)
  if not M.config.sort.folders_first and not M.config.sort.files_first then
    return
  end

  if not a.nodes and b.nodes then
    -- file <> folder
    return M.config.sort.files_first
  elseif a.nodes and not b.nodes then
    -- folder <> file
    return not M.config.sort.files_first
  elseif fallback and a.nodes and b.nodes then
    -- folder <> folder
    return fallback()
  end
end

Yes, we could be tricky with ternaries but readability is more important.

[Akmadan23]

Oh, I didn't consider to completely ignore sort.folders_first but it works so that's fine.

[Akmadan23]

FYI: you're a repo member: you can push a branch to this repo instead of using a fork.

I didn't think about that, next time I'll push a new branch here.

[Akmadan23]

With the last commit I removed the fallback because it's used only once, I think this way is even more simple and clear.

[alex-courtis]

With the last commit I removed the fallback because it's used only once, I think this way is even more simple and clear.

That's great... simple is best.

I'll test this one on the weekend.

[alex-courtis]

Tested:

• all permutations of folders_first and files_first
• sort function

Many thanks for your contribution!