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

Add preview support (#16) #203

Merged
merged 4 commits into from
Jan 7, 2025
Merged

Conversation

danitrap
Copy link
Contributor

@danitrap danitrap commented Dec 16, 2024

Description

Adds preview functionality to sesh for displaying session/directory contents in fzf preview window, this should close #16 .

Features

  • Add new preview command that supports either tmux sessions or directory listing
  • Support for tmux session, config file and directory previews
  • Add ls_command config option for customizing directory listing
  • Expand fzf popup window size and add preview panel
  • Update mocks with mockery v2.50.0

The preview shows:

  • Active tmux session contents via capture-pane
  • Directory listing for configured and zoxide paths
  • Custom ls command support through config

Screenshots

Tmux Session

image

Config Directory

image

Tmuxinator

image

Zoxide

image

fd

image

custom preview_command with bat

image

@joshmedeski
Copy link
Owner

Dude, this is awesome, nice work!

Here is some feedback:

  1. Can you provide some context for how the ls_command configuration works? I couldn't get it to work on my end
  2. I'd prefer to rename ls_command to preview_command and allow any command to be passed (I'll use bat file in some cases)
  3. Does the ls_command work for the default_session config as well?

Overall, this is great work, I don't see anything glaringly wrong with the code itself, just some clarification on the configuration, thanks again.

@danitrap
Copy link
Contributor Author

Dude, this is awesome, nice work!

Thanks, my man @joshmedeski! I love using Sesh and wanted to contribute to give back. Thank you for creating it and for all your hard work.

  1. Can you provide some context for how the ls_command configuration works? I couldn't get it to work on my end

Ah, I knew I was forgetting something—I didn’t document it, sorry about that! I’ll add documentation once we clarify its purpose and figure out how it should be configured.

In essence, ls_command lets you configure an alternative command for ls. For example, in my setup (shown in the screenshots in this PR), I’m using eza with this command:

$ eza --all --git --icons --color=always {path_to_directory}

This gives a nice ls output.

  1. I'd prefer to rename ls_command to preview_command and allow any command to be passed (I'll use bat file in some cases)
  2. Does the ls_command work for the default_session config as well?

I see how this might be confusing. Placing it in the default_session TOML table suggests it’s tied to new sessions or used for file previews.

In reality, it’s a directory previewer configuration. Perhaps we should create a dedicated TOML table for previewer-specific settings? Alternatively, we could move it to the root of the config for clarity. Let me know what you think!

Overall, this is great work, I don't see anything glaringly wrong with the code itself, just some clarification on the configuration, thanks again.

Thank you so much! This is actually my first contribution in Golang, so I’m thrilled it meets acceptable quality standards. I’m usually a TypeScript dev, so this was a fun learning experience.

@joshmedeski
Copy link
Owner

This is how I see the preview_command working:

[default_session]
startup_command = "nvim +GoToFile"
preview_command = "lsd  --group-dirs first -A {path_to_directory}"

[[session]]
name = "tmux config"
path = "~/c/dotfiles/.config/tmux"
startup_command = "nvim tmux.conf"
preview_command = "bat ~/c/dotfiles/.config/tmux/tmux.conf"

It's a preview for the session, so semantically I think it's clear for both default or custom sessions.

The only piece that isn't intuitive is the {path_to_directory} convention, but with clear instructions on the README and I think it will be okay.


Similar to the startup strategies, I recommend creating a default config strategy before the directory

		NewTmuxStrategy(lister, tmux),
		NewConfigStrategy(lister, ls),
                NewDefaultConfigStrategy(config, lister, ls), // config should be passed as a dependency to get the default value
		NewDirectoryStrategy(home, dir, ls),

@danitrap
Copy link
Contributor Author

Oh, that makes sense! That's awesome, I'll work on it. Maybe we can borrow the syntax from fzf which uses {} as a placeholder for the currently selected item.

@danitrap danitrap marked this pull request as draft December 20, 2024 13:53
Replace hardcoded ls command with configurable preview command that
supports
path placeholder. This allows users to customize how sessions are
previewed
with their preferred commands.

The preview command can be set per-session or globally in config, with
fallback to default ls behavior. Commands support {} placeholder which
gets
replaced with the session path.
Add documentation and examples for the new preview command feature in
both default and per-session configurations. Include examples using bat
and eza tools for file and directory previews.
@danitrap danitrap marked this pull request as ready for review December 20, 2024 14:34
@danitrap
Copy link
Contributor Author

Hey @joshmedeski,

I've added a custom preview_command to the per-session configuration and documented the previewer in the README. Let me know if you have any feedback.

Thank you!

@danitrap
Copy link
Contributor Author

Hi @joshmedeski have you had a chance to look at this yet?

@joshmedeski joshmedeski self-requested a review January 7, 2025 04:25
Copy link
Owner

@joshmedeski joshmedeski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, nice work!

@joshmedeski joshmedeski merged commit 0c3480c into joshmedeski:main Jan 7, 2025
4 checks passed
@danitrap
Copy link
Contributor Author

danitrap commented Jan 7, 2025

@joshmedeski thank you so much for merging it. Happy new year!

@danitrap danitrap deleted the 16-preview-support branch January 10, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preview Support
2 participants