Feature/deduplicate sessions

[seflue]

Removes duplicates to already running tmux sessions from the result list.

• removes config sessions with the same name
• removes zoxide entries, where the basename of a path is already the name of a running tmux session.

This PR also introduces a new depedency: samber/lo.

To conveniently work with the combined session list, it also uses the score field, which is currently only used by zoxide to conveniently order sessions.

[joshmedeski]

Nice work! Give me the weekend to look it over, thanks for putting this together.

[seflue]

This is a bit different than what was originally implemented and in the current stage it might give a different order for config and tmux sessions than originally provided by the source strategy - if the OrderedIndex has a different order than iterating over the Directory map would give us.

My question here is: Why are we maintaining a separate "OrderedIndex" anyway? Wouldn't it be better to have the sorting criterion as a property of the SeshSession and sort the session slice by this criterion?

I am currently achieving this by using the Score also for the other source types, not only Zoxide entries. Instead of returning the OrderedIndex together with the Directory map, it could make sense to populate the Score field already in the strategy methods and just add the source-specific offset here. Alternatively we could leave the Score for Zoxide only, but enhance the sorting function by first looking at the index of the source in srcStrategies and take the Score only as second criterion. The offset method I am using right now could be seen as a kind of clever hack ... I think it's a matter of taste and I would be fine with both approaches.

[joshmedeski]

The way I originally wrote it in bash and sesh v1 was to go through each list (ex: tmux), then pass the tmux list output to the next list (ex: config), and do sorting and filtering off the previous list's data.

Here are some rules I think would be best in following:

1. tmux sessions are always shown first (in order of last attached). I think this is the most predictable pattern for active sessions.
2. Replacing zoxide paths with configs with the matching path. It allows configs to always be prioritized over zoxide results. Especially since you could have multiple configs using the same root path if you want.
3. Only sort non-tmux sources by score, which will mix custom configs and zoxide results together. To me, this would be a positive improvement to how sesh current works (just listing custom configs how they're defined)
4. Allowing people to opt-in or opt-out of the feature that overlapping results (tmux sessions, custom configs, or zoxide results with the same base path). Perhaps we let people opt-in since users will already be used to the current functionality.

Hope those pointers help, I'd be happy to do some pair programming together if you're interested!

[ccoVeille]

While I'm a fan of what @samber (hi 👋) made with his lib when it came out.

Are you sure the code changes you are doing cannot be fine with maps and slices std lib packages?

This could avoid importing the lib

[seflue]

While I'm a fan of what @samber (hi 👋) made with his lib when it came out.

Are you sure the code changes you are doing cannot be fine with maps and slices std lib packages?

To be honest, I am not a Go expert. I have mainly used other programming languages in the past, which either have these kinds of abstractions like the lo library gives us built in, or have a similar library available to make this possible.

So I did a little research on what we can do with the std-library and I am not satisfied.

• The idiom I need to use to replace lo.Map as well as lo.GroupBy is a for loop. I know - gophers love their for-loops, but in this case it blurs the intent and makes the code harder to read.
• Instead of lo.Filter I would have to reverse my logic and use slice.DeleteFunc. This is mostly equivalent, although filter is the more common abstraction for most programmers.
• As soon as I need some higher level abstractions like a FlatMap, I am back to nested for-loops.

This could avoid importing the lib

While I know and agree with Rob Pike's Go proverbs - "A little copying is better than a little dependency" could be applicable here - I also appreciate Dijkstra's "The purpose of abstraction is not to be vague, but to create a new semantic level in which one can be absolutely precise.". And that is exactly what I am trying to achieve here - I want to use the precise and established language of set operations to communicate in a clear way the purpose of the code. And I see little value in avoiding a dependency just for the sake of avoiding it, especially for a basic and widely used package like samber/lo.

Full disclosure: I took my first steps in Go a few days ago by adding a feature to lazygit. This is where I became aware of samber/lo, because they are using it all over the codebase. And I was quite happy, because it gave me the opportunity to think in abstractions I was already familiar with, without having to be a Go expert. Before, my perception of Go was "they are always using tedious for loops".

In the end, @joshmedeski has to decide which style he prefers in his project.

[ccoVeille]

Thank you for your detailed reply. It givesva lot of context.

I raised the point because slices and maps packages are pretty new and not widely known and used.

Based on your reply, I can tell and made your own researches. So I'm fine with the code you provided.

But let's wait for a maintainer

[joshmedeski]

Great conversation here, thanks for asking good questions here @ccoVeille. I had similar opinions on introducing new dependencies.

Overall, my main concern is maintainability and performance. I'll take some time this week to look at the code in-depth and do some measurements for the cost of introducing this new package.

I LOVE the idea of adding scoring to non-zoxide list results, something I was already thinking about.

Thanks again and give me about a week to look this over and provide more detailed feedback.

[joshmedeski]

Hey @seflue, here is some initial feedback on how the feature is working for me.

1. This feature doesn't seem to work with git repos, git worktrees, or any paths that have to substitute periods for underscores

2. Every time I open a sesh picker, both the tmux sessions and configuration items appear to be randomly sorted each time. I like matching them to the zoxide score, but it doesn't appear to be doing that

[seflue]

Hi @joshmedeski, thanks for the feedback.

1. The issue with the underscore is interesting, I have to take some time to have a look into that. Actually I wasn't aware, that sesh is replacing the . with an _ - the reason was, that I probably never have created a session on a hidden folder. Is the dot replaced by sesh? Maybe you can point me to a place in the code where this is done.
   Edit: It seems, that the issue is gone with my fix, see below.

2. For the issue with the ordering I just pushed a fix. I am now attaching the score directly when generating the tmux list and derive it from the LastAttached unix time stamp. The way I formerly implemented that was more of an ugly hack (which obviously did not work).

I also introduced a debug command line argument to show the scores in the output. We don't have to keep it, but it helps to understand the order.

[joshmedeski]

I've pulled in the updates and tested it.

1. The folder matching doesn't seem to be working

2. I'm getting a lot of duplicates here, I'd expect my named sessions to match the paths and be hidden

3. My custom configs are all showing up at the very end of the list, and many of these should show up toward the very top on top of my of my zoxide results.
   

Perhaps I don't fully understand how these changes are supposed to work, but I personally want all my custom configs to show up before the zoxide results.

[szinn]

I've added a simple filter PR - #216 - that will remove any duplicates out of the list based on the Path entries.

[seflue]

Thanks @szinn. I think, that your solution is the much more idiomatic solution.

I have to admit, that I on one hand wanted to much (functional style instead of for loops, stable sortable order), to be more open for configurability, on the other hand hesitated from changing the individual listers. I already thought about a normalized score for each lister to stabilize the sorting.

@joshmedeski: The reason for the duplicates was, that I compared the names, not the paths. Sometimes I have multiple sessions for one folder. But in recap I would say: Let's go with @szinn solution. It does exactly what most users want, it doesn't break the originally intended logic, it avoids the dependency (although I still like the functional approach) and it is plain, simple and idiomatic.

Even the feature requested in #214 would not really bloat the code.

Thanks again @szinn, also for reminding me, that less is sometimes more. 👍

[szinn]

@seflue - I have made the change below to my tmux.conf file and defaulted the -d everywhere except the ^a option which shows the duplicates

Thanks for the nice comment - much appreciated!

bind-key "t" run-shell "sesh connect \"$(
  sesh list --icons -d | fzf-tmux -p 80%,70% \
    --no-sort --ansi --border-label ' sesh ' --prompt '⚡  ' \
    --header '  ^a all ^t tmux ^g configs ^x zoxide ^d tmux kill ^f find' \
    --bind 'tab:down,btab:up' \
    --bind 'ctrl-a:change-prompt(⚡  )+reload(sesh list --icons)' \
    --bind 'ctrl-t:change-prompt(🪟  )+reload(sesh list -d -t --icons)' \
    --bind 'ctrl-g:change-prompt(⚙️  )+reload(sesh list -d -c --icons)' \
    --bind 'ctrl-x:change-prompt(📁  )+reload(sesh list -d -z --icons)' \
    --bind 'ctrl-f:change-prompt(🔎  )+reload(fd -H -d 2 -t d -E .Trash . ~)' \
    --bind 'ctrl-d:execute(tmux kill-session -t {2..})+change-prompt(⚡  )+reload(sesh list -d --icons)' \
    --margin 8% \
    --preview-window 'right:55%' \
    --preview 'sesh preview {}'
)\""

[seflue]

@szinn: Fun fact: I actually had this changes to the tmux config as debugging commit already in store, because I added a -d debugging flag in my own branch. So switching to your solution was very smooth: just checking out your PR and switch a branch in my dotfiles.