fix: respect 'insert_after_current' and 'insert_at_end' after a custom sort is set

[AThePeanut4]

This PR fixes two issues related to behaviour with a custom buffer sort order that have been bothering me for a while.

As described in #706, if sort_by is "insert_after_current" or "insert_at_end", and you move a buffer manually using :BufferLineMoveNext or :BufferLineMovePrev, new buffers are no longer inserted after the current buffer or at the end, but rather based on their ID.
This fixes this by moving the logic out of the sort function, and into what was the get_updated_buffers function. There, while sorting the buffers according to the order in custom_sort, it will also ensure that new buffers (ie those that do not appear in custom_sort) are put where they should be. It then updates custom_sort to now contain those new buffers.

The second issue is that restoring the custom sort order from a saved session didn't work correctly most of the time, because it was storing buffer ids which are not preserved by :mksession. The fix is to store the full paths of the buffers in g:BufferlinePositions instead. vim.json is required to stringify the list as :mksession only stores string/number values.

This is the first time I've done anything substantial with a Neovim plugin, so I apologise if I've done something very wrong. I've only really tested this in my own setup, and I haven't done anything to the tests, because I don't really know how all that works. Feel free to make any changes you think are needed.

[akinsho]

@AThePeanut4 I strongly suggest separating both changes out into separate PRs so that each change can be assessed and merged separately. Currently this PR introduces to many new changes at once. The change to fix the behaviour with session I think is most welcome. The change that moves sorting logic into the buffer fetching function will need a lot more conversations since it puts logic concerning specific sort strategies literally at the source where all buffers are fetched which I believe is a pretty big problem

[akinsho]

A change that involves moving sorting specific logic out of the sorting specific module into the module that resolves all buffers tbh is not really something I'll accept as this co-mingles logic that should absolutely remain separate. I think this needs further thought. I haven't had a thorough read yet because all in all this PR is too busy to make sense of as is

[AThePeanut4]

In my opinion, "insert_after_current" and "insert_at_end" aren't really "sort methods" at all - they don't really "sort" anything. Manually calling require("bufferline").sort_by("insert_after_current") as the user doesn't make any sense.

So in that sense, they shouldn't really be set under sort_by in the config. However, specifying "insert_after_current" and "insert_at_end" in the config would still need to be mutually exclusive with the other sorting methods, so keeping it under sort_by in the config is okay and it keeps backward compatibility.

I see them as much more of an "automatic custom sort" function, where they determine where to add new buffers to an already existing custom sort order. This is why I moved the logic out of the sorting module and into the buffer module - I think it is a much better fit there.

[akinsho]

@AThePeanut4 so this seems like something that could quickly become a back and forth of opinions. So I will start with some givens I think. The plugin as it is already exists in the current form (used by X number of people +/- historical contributors) with both of these things as sort mechanisms so this is a practical reality in that sense regardless of opinion.

Also they are sort methods because ultimately none of any of the sorting methods are native to neovim in anyway. None of this is how neovim itself handles or arranges buffers. It essentially presents them as a list in order of the buffer ID which is unique per session.

So all attempts to change the "natural" neovim order of buffers to my mind are sorting methods particularly because to arrive at the desired arrangement we literally need to sort the list from the way neovim has it to the preferred way.

Regarding them just being ways to insert new buffers after choosing a sort method I don't think I follow this or it makes much sense to me. If you say you want your buffers sorted by name then inserting_after_current|end are definitely not

an "automatic custom sort" function, where they determine where to add new buffers to an already existing custom sort order.

In that case sort after current or end has nothing to do with the requested sort order which was alphabetical etc. Just to really re-enforce this point a user can create their own sorting mechanism and sorting does not at all imply that new buffers should go to the end or after the current these are completely separate choices that should not apply to all sorting since many sorting mechanisms might put the new buffer at any position which fits its sorting strategy.

Also it's important to note that custom_sort specifically refers to when the user has manually shifted around the buffers which can very easily be something a user never does e.g. myself.

[AThePeanut4]

They're not "sort methods" in the sense that they operate in a completely different way to the other sorting methods (alphabetical, etc). They take the previous order and add new buffers to it in the correct position, rather than sorting based on some property of the actual buffer. Saying "I want to sort my buffers by 'inserting new buffers after the current buffer'" sounds strange.

They're also different because a "insert_after_current|end" "sort" will not survive through a session, as since there is no "previous order" it has to fall back to just sorting by buffer ID.

In that case sort after current or end has nothing to do with the requested sort order which was alphabetical etc. Just to really re-enforce this point a user can create their own sorting mechanism and sorting does not at all imply that new buffers should go to the end or after the current these are completely separate choices that should not apply to all sorting since many sorting mechanisms might put the new buffer at any position which fits its sorting strategy.

This is why I said sort_by and "insert_after_current|end" are mutually exclusive - a sorting mechanism (name, custom sort function, etc) could of course put a new buffer anywhere, not just after the current buffer or at the end. It only makes sense to have "insert_after_current|end" if you're not sorting by something else.

The way I see it, as a user you have a choice:

• Do you want to have the buffers always sorted according to some property of the buffer (name, etc), which is what I'm referring to as a "sort", where the displayed order of the buffers is a pure function of the buffers themselves. New buffers will just be placed according to the sort property, so "insert_after_current|end" does not make sense. Here the order does not need to be stored as state or saved in the session file, since it can be reconstructed from the actual buffers.
• Or do you want to maintain some custom order (ie a custom_sort) where the buffers are always displayed in this order. Then when a new buffer comes along, you can choose where in the custom order you want it to be added - after the current buffer or at the end of the list. In this case the order needs to be stored as state (custom_sort) and saved in the session file, as it cannot be reconstructed from the buffers themselves.

When you use :BufferLineMoveNext or :BufferLineMovePrev, you are then essentially saying "I no longer want to have my buffers always sorted according to property X, I want to now use a custom order". Also if you manually sort the buffers with require("bufferline").sort_by(X) then you are saying "I want to now use a custom order initialized to the order sorted by property X."

[AThePeanut4]

@AThePeanut4 I strongly suggest separating both changes out into separate PRs so that each change can be assessed and merged separately. Currently this PR introduces to many new changes at once. The change to fix the behaviour with session I think is most welcome. The change that moves sorting logic into the buffer fetching function will need a lot more conversations since it puts logic concerning specific sort strategies literally at the source where all buffers are fetched which I believe is a pretty big problem

Alright I've split off the second part, this PR is now just for the first change.

[akinsho]

@AThePeanut4 I should firstly start by saying thanks for raising this PR.

I think it would be good to really get a clear sense of the specific problem it is trying to solve stated quite plainly since I think it seems we are likely not using the same language in the same sense to describe the same things.

From what I can gather the main objective of this PR is to try to maintain the insert_after ordering whilst also allowing users to move buffers to specific positions since currently having a custom_sort overrides a previous sort_by for simplicity frankly.

If this is the case then I think a good approach would be start with looking inside of the sorters.lua module at whether the two strategies can be combined and importantly to try to avoid doing this in a place where it ends up potentially the position where buffers are fetched.

To be absolutely clear fetching buffers should essentially just return the list from neovim with some filtering based on loaded state etc. I appreciate the custom sort was implemented there historically but frankly that's a bit of tech debt. That function should primarily just fetch buffers not sort them.

[AThePeanut4]

From what I can gather the main objective of this PR is to try to maintain the insert_after ordering whilst also allowing users to move buffers to specific positions since currently having a custom_sort overrides a previous sort_by for simplicity frankly.

You are basically correct, I essentially want to be able to maintain a custom sort order (ie :BufferLineMoveNext or :BufferLineMovePrev) while still having new buffers be inserted after the current one or at the end. This is essentially just issue #706. Additionally, when "insert_after_current|end" is set, the order is not persisted in a session file, which this fixes.

To be absolutely clear fetching buffers should essentially just return the list from neovim with some filtering based on loaded state etc. I appreciate the custom sort was implemented there historically but frankly that's a bit of tech debt. That function should primarily just fetch buffers not sort them.

That makes sense. I think what I'll do is move the custom sort logic into sorted.lua.