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 config setting for showing tabs #1755

Merged
merged 2 commits into from
Mar 1, 2024
Merged

Conversation

digital-cuttlefish
Copy link
Contributor

From #1754

Yet to do local testing, but this is what I had in mind.

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

@tessus
Copy link
Contributor

tessus commented Feb 25, 2024

I like the idea, but I think the name of the config option is a bit off. It sounds that atuin won't show any tabs anymore. If you don't read the explanation that is.

I'd opt for a name like show_tab_info or something along those lines. But that's just me.

@digital-cuttlefish
Copy link
Contributor Author

Oh, I actually wanted the tabs themselves to be hidden.
Where should I look for that?

@tessus
Copy link
Contributor

tessus commented Feb 29, 2024

No, your code is perfectly fine afaik by looking at it. I haven't tested it, but it looks fine.
I only mentioned that the option name you used show_tabs is a bit ambiguous - at least for me. But maybe I am wrong. Let's see what the devs have to say.

Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

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

I get @tessus' point about show_tabs being a little ambiguous, but I'm happy with this as-is due to how it matches nicely with show_help.

If you could also make a PR to the docs, that would be great! https://github.com/atuinsh/docs

Seeing as this is your first time contributing, if you would like a holographic contributors-only Atuin sticker, then please fill out this form!

We do also have a Discord if you'd like to ask any questions, or just fancy hanging out!

@ellie ellie merged commit 9933220 into atuinsh:main Mar 1, 2024
15 checks passed
@atuin-bot
Copy link

This pull request has been mentioned on Atuin Community. There might be relevant details there:

https://forum.atuin.sh/t/feedback-playing-with-inspector/218/2

@digital-cuttlefish
Copy link
Contributor Author

atuinsh/docs#47

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.

4 participants