Skip to content

proof of concept: allow passing --inline from channel config #661

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ascandella
Copy link
Contributor

📺 PR Description

Partially implements: #641

This is a first draft at implementing UI configuration from channel configs

I haven't implemented tests, nor have I implemented width/height yet. Wanted to get feedback on whether this approach was acceptable first. If you're down with this approach, I'll add tests and implement the rest

Demo:

demo

Checklist

  • my commits and PR title follow the conventional commits format
  • if this is a new feature, I have added tests to consolidate the feature and prevent regressions
  • if this is a bug fix, I have added a test that reproduces the bug (if applicable)
  • I have added a reasonable amount of documentation to the code where appropriate

@alexpasmantier
Copy link
Owner

First of all thanks so much for showing interest in the project.

Putting the implementation aside, I'm actually really curious to have feedback on the idea itself which I'm not yet entirely convinced of.

Having configurable --inline per-channel could mean several things too:

  • do we honor the setting each time a user changes channels from the remote? (which implies potentially switching back and forth between terminal modes with the alternate screen)
  • or does the setting only work when the channel is invoked via the cli?

This can get rather tricky and I'm not sure it really brings any real user value either when I think of it... 🤔

wdyt?

@ascandella
Copy link
Contributor Author

Great questions. I'm not too sure how other people use tv, but I can speak to my usage and my thoughts on this.

I don't use the channel remote at all-- I use tv the same way I use telescope inside neovim, where I know what kind of search I'm going to perform before I launch the tool. And depending on the search type, I have a specific set of configuration I use that differs. Here is my config for telescope.

Conceptually I'd like to use tv the same way. Of course I could define shell aliases that launch it with the correct options based on the type of search I'm doing, but it feels like having the ability to say "when I'm searching git branches, I know I'm not going to need fullscreen so don't even try".

As for whether we honor that when switching: I'd vote no. Not only would that be visually jarring but it seems a bit counterintuitive to me, not to mention the extra code it would mean supporting.

But I'm a relatively new tv user, so I'm interested in hearing your thoughts (as well as any others that come across this).

@alexpasmantier
Copy link
Owner

I agree with the above. Let's leave this open for now and maybe start a discussion on GitHub or on discord to collect feedback from other users to get it right. Anyways, the implementation looks fine and should be close enough to the target if we decide to proceed with that 👍

@alexpasmantier
Copy link
Owner

Oh and thanks so much for the sponsorship, it means a lot 🙂🙏

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.

2 participants